Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc_cfg does not have a correct effect on use items referencing other public items #85043

Open
nagisa opened this issue May 7, 2021 · 1 comment
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-bug Category: This is a bug. F-doc_cfg `#![feature(doc_cfg)]` T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@nagisa
Copy link
Member

nagisa commented May 7, 2021

Consider the following code:

#[doc(cfg(feature="include_file"))]
pub mod include_file {
    pub fn generated() {}
}

pub use include_file::*;
pub use include_file::generated;
pub use include_file::generated as renamed;

The produced documentation looks like this:

An image of rustdoc output for the crate root, showing 3 reexports, use include_file::* which has a doc_cfg badge, and use include_file::generated as well as use include_file::generated as renamed which don't

I think we should be showing the doc_cfg badge consistently here: either on all of the re-exports or none of them. I would probably lean towards "none" in this particular case, in part because it would be easier to implement properly situations where re-exports don't share the features of the modules that are being re-exported. For instance:

#[doc(cfg(feature="include_file"))]
pub mod include_file {
    pub fn generated() {}
}
#[doc(cfg(feature="reexport_glob"))]
pub use include_file::*;
#[doc(cfg(feature="reexport_generated"))]
pub use include_file::generated;
#[doc(cfg(feature="reexport_renamed"))]
pub use include_file::generated as renamed;

which generates exactly the same documentation page as shown above. I believe in this case we should show exactly the specified cfg badges, rather than attempting to infer from what items are being re-exported.

(related #83428, #43781)

@nagisa nagisa added F-doc_cfg `#![feature(doc_cfg)]` T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. labels May 7, 2021
@nagisa nagisa changed the title doc_cfg does not have any effect on the re-exports doc_cfg does not have any effect on the re-export items May 7, 2021
@nagisa nagisa changed the title doc_cfg does not have any effect on the re-export items doc_cfg does not have a correct effect on use items referencing other public items May 7, 2021
@est31
Copy link
Member

est31 commented May 7, 2021

I believe in this case we should show exactly the specified cfg badges

I agree.

You can only use something from a module if the module is available, which means that if there are multiple implementations of a module gated by different cfg attributes, but the use is free of any cfg, there should be no inheritance from the module that rustdoc ran for. Conversely, if the pub use contains a more restricted cfg than the module, you would want to document that restricted cfg instead of the module's cfg, because the use would simply not be available in some cases.

TLDR: pub use should only show what attributes on the use itself contain.

@camelid camelid added the C-bug Category: This is a bug. label Dec 15, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Dec 15, 2023
…eexport, r=rustdoc

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

Fixes rust-lang#112881.

## Explanations

When re-exporting things with different `cfg`s there are two things that can happen:

 * The re-export uses a subset of `cfg`s, this subset is sufficient so that the item will appear exactly with the subset
 * The re-export uses a non-subset of `cfg`s (e.g. like the example I posted just above where the re-export is ungated), if the non-subset `cfg`s 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 `cfg`s 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 `cfg`s on the path up until the glob can be removed, and only `cfg`s on the globbed item itself matter), for non-inlined re-exports see rust-lang#85043.

cc `@Nemo157`
r? `@notriddle`
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Dec 15, 2023
Rollup merge of rust-lang#113091 - GuillaumeGomez:prevent-cfg-merge-reexport, r=rustdoc

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

Fixes rust-lang#112881.

## Explanations

When re-exporting things with different `cfg`s there are two things that can happen:

 * The re-export uses a subset of `cfg`s, this subset is sufficient so that the item will appear exactly with the subset
 * The re-export uses a non-subset of `cfg`s (e.g. like the example I posted just above where the re-export is ungated), if the non-subset `cfg`s 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 `cfg`s 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 `cfg`s on the path up until the glob can be removed, and only `cfg`s on the globbed item itself matter), for non-inlined re-exports see rust-lang#85043.

cc `@Nemo157`
r? `@notriddle`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-bug Category: This is a bug. F-doc_cfg `#![feature(doc_cfg)]` T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants