Don't merge cfg and doc(cfg) attributes for re-exports

[GuillaumeGomez]

Fixes #112881.

Explanations

When re-exporting things with different cfgs there are two things that can happen:

• The re-export uses a subset of cfgs, this subset is sufficient so that the item will appear exactly with the subset
• The re-export uses a non-subset of cfgs (e.g. like the example I posted just above where the re-export is ungated), if the non-subset cfgs are active (e.g. compiling that example on windows) then this will be a compile error as the item doesn't exist to re-export, if the subset cfgs are active it behaves like 1.

Glob re-exports?

This only applies to non-glob inlined re-exports. For glob re-exports the item may or may not exist to be re-exported (potentially the cfgs on the path up until the glob can be removed, and only cfgs on the globbed item itself matter), for non-inlined re-exports see #85043.

cc @Nemo157
r? @notriddle

[Nemo157]

The test failures are expected, my argument in the issue is that those tests are wrong. (I didn't go looking to see whether there's other tests that I'd expect to fail but didn't).

[GuillaumeGomez]

Sorry should have precised: just realized that this wasn't ready yet. I'm finishing another PR first then coming back to this one.

[GuillaumeGomez]

It's now ready. So in any case, this will need to go through an FCP. But I'm personally not a big fan of this change:

• deprecated/unstable and equivalents are "inherited" by re-exports, so it's not coherent with this change.
• Unlike other items, re-exports are a bit special since it's a same item with potentially extra conditions, extra documentation and another name. I think all such information should be available on the end re-export. Even if a missing cfg will trigger a compilation failure, I think it'd be a better user experience to show all needed requirements directly on the re-export instead of discovering them one by one through potentially multiple failures.

Anyway, that was my two cents on this. We'll see what the others think about it.

[est31]

Even if a missing cfg will trigger a compilation failure

The compilation failure will be triggered at the reexport site, or in other words, I'd consider it a bug if the item is not provided.

Another thing that speaks in favour of this PR is that there is often patterns like:

#[cfg(target_os = "linux")]
mod impl {
pub fn foo() { /* impl for linux */ }
}

#[cfg(target_os = "windows")]
mod impl {
pub fn foo() { /* impl for linux */ }
}

use impl::foo;

Here, there should ideally be no cfg documented as there is implementations for all operating systems.

[GuillaumeGomez]

But in this case, there is no cfg on foo directly, so nothing will be displayed, even currently. The equivalent would be (even if it doesn't compile but to get an idea):

#[cfg(target_os = "linux")]
pub fn foo() { /* impl for linux */ }

#[cfg(target_os = "windows")]
pub fn foo() { /* impl for windows */ }

pub use self::foo as reexport;

[Nemo157]

But in this case, there is no cfg on foo directly, so nothing will be displayed, even currently.

No, cfg's are merged down onto the item before the re-export is inlined

#![feature(doc_auto_cfg)]

#[cfg(target_os = "linux")]
mod impl_ {
    pub fn foo() { /* impl for linux */ }
}

#[cfg(target_os = "macos")]
mod impl_ {
    pub fn foo() { /* impl for darwin */ }
}

pub use impl_::foo;

That doesn't really matter though, your example showing the cfg on the inlined re-export would also be wrong, since the public item is not gated on any platform the crate compiles on. (Except it's not inlined since the re-exported items are pub so my arguments (and hopefully this change) don't apply).

[Nemo157]

(I would suggest maybe doing FCP in the issue since that's where I documented my rationale for the change).

[GuillaumeGomez]

You could also start it here and add an explanation text too. As you prefer.

[Nemo157]

@rfcbot pr merge

My reasoning for this change from the issue:

When re-exporting things with different cfgs there are two things that can happen:

1. The re-export uses a subset of cfgs, this subset is sufficient so that the item will appear exactly with the subset
2. The re-export uses a non-subset of cfgs (e.g. like the example I posted just above where the re-export is ungated), if the non-subset cfgs are active (e.g. compiling that example on windows) then this will be a compile error as the item doesn't exist to re-export, if the subset cfgs are active it behaves like 1.

This only applies to non-glob inlined re-exports, for glob re-exports the item may or may not exist to be re-exported (potentially the cfgs on the path up until the glob can be removed, and only cfgs on the globbed item itself matter, but I haven't thought through all the details), for non-inlined re-exports see #85043.

[rfcbot]

Team member @Nemo157 has proposed to merge this. The next step is review by the rest of the tagged team members:

• @GuillaumeGomez
• @Manishearth
• @Nemo157
• @aDotInTheVoid
• @camelid
• @jsha
• @notriddle

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[bors]

☔ The latest upstream changes (presumably #114115) made this pull request unmergeable. Please resolve the merge conflicts.

[GuillaumeGomez]

Fixed merge conflict.

[bors]

☔ The latest upstream changes (presumably #117875) made this pull request unmergeable. Please resolve the merge conflicts.

[GuillaumeGomez]

Fixed merged conflict.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@bors r=rustdoc

📌 Commit 823148f has been approved by rustdoc

It is now in the queue for this repository.

⌛ Testing commit 823148f with merge 4d1bd0d...