doc_cfg hints not shown on reexports from private modules

[MarijnS95]

Hi @GuillaumeGomez! I am currently revisiting the idea to omit cfg/doc_cfg attributes when the parent (impl block or mod in its entirety) already has a version constraint that is at least as strict. This was sparked after going through https://doc.rust-lang.org/std/os/index.html, where modules and types do not have #[doc(cfg(unix))] but seem to inherit the constraint from their parent module, solving exactly the issue you were describing.
In other words, we have version information on the module, any modules within it, and any struct/trait that has its own page (right at the top, and on the module overview page) - without recursively duplicating #[doc(cfg())] etc 🥳

I have hence rebased those commits back and regenerated, and, to no surprise, the features are indeed propagated into the files/modules... but not in the way I expected.

"Fortunately" the same issue seems to be affecting master already. doc(cfg) is currently only used on mods and fns, not on structs in ie. glib::wrapper!. As it seems rustdoc is not able to put these feature requirement labels on items in a private module (mod auto;) that are then re-exported (pub use auto::*;) - even if the same version constraint is on both. Let's take for example gdk::DeviceTool. On master it has a feature constraint on the mod and fns, and looks like this (with the following rustc nightly):

me:gtk-rs/ $ rustc +nightly -Vv
rustc 1.53.0-nightly (5d04957a4 2021-03-22)
binary: rustc
commit-hash: 5d04957a4b4714f71d38326fc96a0b0ef6dc5800
commit-date: 2021-03-22
host: x86_64-unknown-linux-gnu
release: 1.53.0-nightly
LLVM version: 12.0.0

There's no version constraint on the module overview page:

On the page for that type, there is no feature requirement on the struct, but there is on the fns:

Nothing changes after regenerating with the commits linked above. All superfluous #[cfg] and #[doc(cfg())] are now gone making the code a little more readable, though.

However, if the constraint is added to the struct directly we finally get what we want:

Note that #[doc(cfg)] has been added back to the fns but do not show in the docs, to illustrate that they are coalesced into the requirement on the struct as a whole 🥳

In closing:

• I should submit the above commits. They do not improve the documentation situation, but do make the code a little more readable with all the duplicate #[cfg]'s gone;
• Create an issue about this on the rust tracker. rustdoc: doc_cfg hints not shown on reexports from private modules rust-lang/rust#83428: It seems like rustdoc needs to learn how to combine the #[doc(cfg)] of the module and the reexport? I could not find any open issue for this but there are too many to check them all;
• Temporary: emit doc(cfg) directly on the type (ie. inside glib::wrapper!) to get the desired result in the third and fourth picture.

What do you think?

[MarijnS95]

To illustrate this issue differently I have minimal reproducible example, including comments explaining the situation:

#![feature(doc_cfg)]

#[doc(cfg(feature = "foobar"))]
pub mod imp_pub {
    /// Feature on `struct` in public module shows, despite no `doc_cfg` directly on the `struct`
    pub struct BarPub {}
}
// This only shows Re-exports, but doesn't place the type in the top-level module
#[doc(cfg(feature = "foobar"))]
pub use crate::imp_pub::*;

#[doc(cfg(feature = "foobar"))]
mod imp_priv {
    /// Feature on `struct` in private module is never shown
    pub struct BarPriv {}
    impl BarPriv {
        /// Oddly enough the feature guard _is_ shown here
        pub fn test() {}
    }
}
#[doc(cfg(feature = "foobar"))]
pub use crate::imp_priv::*;

#[doc(cfg(feature = "foobar"))]
mod imp_priv_2 {
    /// In a private module `doc_cfg` has to be placed directly on the `struct`
    #[doc(cfg(feature = "foobar"))]
    pub struct Bar {}
    impl Bar {
        /// The explicit feature guard is hidden here because it is _implied_ from the surrounding [`Bar`] type
        #[doc(cfg(feature = "foobar"))]
        pub fn test() {}
    }
}
#[doc(cfg(feature = "foobar"))]
pub use crate::imp_priv_2::*;

This shows:

And:

But we really want BarPriv to look like Bar in the module overview, and like Bar in its dedicated page:

[GuillaumeGomez]

In closing:

• I should submit the above commits. They do not improve the documentation situation, but do make the code a little more readable with all the duplicate #[cfg]'s gone;
• Create an issue about this on the rust tracker. It seems like rustdoc needs to learn how to combine the #[doc(cfg)] of the module and the reexport? I could not find any open issue for this but there are too many to check them all;
• Temporary: emit doc(cfg) directly on the type (ie. inside glib::wrapper!) to get the desired result in the third and fourth picture.

What do you think?

Well first of all: this is awesome! Thanks a lot for doing this!

My main question now is: will rustdoc remove this feature before that this one gets merged or not? Well, I suppose not. For reference, here is the work in progress to remove the need for doc_cfg: rust-lang/rust#79341

For the combination of the attributes, I'm surprised that rustdoc doesn't handle it. I think it'd be better to open an issue there so it doesn't get lost as you suggested.

But beyond that, I'm curious of the actual code. Don't have much suggestions for the moment but the end result definitely looks great.

[MarijnS95]

Well first of all: this is awesome! Thanks a lot for doing this!

Thanks! Actually I have to thank you for working on rustdoc, it is truly awesome and elegant, both to write docs as well as to read them!

My main question now is: will rustdoc remove this feature before that this one gets merged or not? Well, I suppose not. For reference, here is the work in progress to remove the need for doc_cfg: rust-lang/rust#79341

You mean implying doc(cfg()) based on cfg()? That's easily a separate change, it doesn't really matter which one is merged first to gir, we'll just regenerate. I can anyway submit the deduplication straight away since it makes no functional difference.

For the combination of the attributes, I'm surprised that rustdoc doesn't handle it. I think it'd be better to open an issue there so it doesn't get lost as you suggested.

I'll make a new issue based on the info above.

But beyond that, I'm curious of the actual code. Don't have much suggestions for the moment but the end result definitely looks great.

There's no actual code yet, nor should there be. The third point (**Temporary**: emit doc(cfg) directly on the type ...) is - as the name implies - only temporary until the rustdoc combination is fixed. If that can happen quickly this should work properly OOTB!

[GuillaumeGomez]

You mean implying doc(cfg()) based on cfg()? That's easily a separate change, it doesn't really matter which one is merged first to gir, we'll just regenerate. I can anyway submit the deduplication straight away since it makes no functional difference.

Considering that doc(cfg()) is a nightly only feature, it's very likely that it'll be a hard error once removed. It'd be pretty problematic for our end users to suddenly have a compilation error out of nowhere because of a remove attribute from the compiler. This is why I'm a bit wary about it. But I suppose we can handle it by just making a minor release if this really happens?

[MarijnS95]

Considering that doc(cfg()) is a nightly only feature, it's very likely that it'll be a hard error once removed. It'd be pretty problematic for our end users to suddenly have a compilation error out of nowhere because of a remove attribute from the compiler. This is why I'm a bit wary about it. But I suppose we can handle it by just making a minor release if this really happens?

Since it has been a nightly feature that tradeoff must have already been made when its use was introduced here. End-users are probably well aware that they have to build the docs with nightly anyhow? Are users doing that anyway or simply compiling their own programs without dox feature, relying on those hosted online?

Anyway, that's nothing that this issue nor the PR changes.

[sdroege]

End-users are probably well aware that they have to build the docs with nightly anyhow?

I think that's the important part here. We already require nightly for building the docs anyway, this PR does not make the situation worse in that regard AFAIU.

I don't like that we require nightly for building the docs, but that was introduced a few weeks ago already.

[MarijnS95]

this PR does not make the situation worse in that regard AFAIU.

Correct - if anything there are now less doc(cfg()) attributes in the code, but that doesn't matter too much if you can't have any at all.