rustdoc: correctly propagate cfgs for glob reexports

[lcian]

View all comments

This fixes some cases of rustdoc not surfacing the correct required feature combination when using #[cfg(feature = ...)] in combination with glob reexports.

See the example cases included as tests.

Here's the generated HTML for those test cases before this change:

and after:

My understanding is that this was basically an off-by-one type of oversight.
We need to update is_inline before calling get_all_import_attributes to immediately reflect that the outermost import we're processing shall indeed be considered as inline, as it's a glob.
Otherwise, we will end up calling add_without_unwanted_attributes with is_inline == false on the "intermediate" re-export, which will skip propagating its cfgs.

I believe this fixes #96166, even though the description there is not super clear on the exact test case they used (there's a snippet but the author says their code is actually different).

Refs #43781
@rustbot label +F-doc_cfg
@rustbot r? @GuillaumeGomez

[GuillaumeGomez]

This is the expected behaviour as described in the rustdoc book: https://doc.rust-lang.org/rustdoc/write-documentation/re-exports.html?highlight=glob#attributes

Although I don't mind changing it. What do you think @rust-lang/rustdoc-internals ?

[yotamofek]

This change absolutely makes sense to me.
The re-exported item will only be visible if the re-export's cfg's predicate is also up-held.

And I might be missing something, but it seems that according the rustdoc book, the current behaviour is wrong for glob re-exports?

These rules also apply if the item is inlined with a glob re-export:

(BTW, the book also has an instance where it says #[cfg(a, b, c)], but I think it should say #[cfg(all(a, b, c))]?)

[GuillaumeGomez]

Oh you're right. Somehow I read the opposite... Sorry about that @lcian.

And yeah, the book has a typo. Feel free to send a PR, otherwise I'll do it once I'm done with my current work.

[Urgau]

Since the re-export's cfg's predicate must also be up-held for the item to be accessible, the change also makes sense to me.

In general I think the default behavior should be the least surprise and this behavior (with/without #[doc(inline)]) seems a bit surprising to me.

[lcian]

Ah, I thought that the CI failures were a GH issue so I retriggered the runs, but looks like they're actually legit and my tests are failing here in CI.
Any advice to debug this? The tests pass locally on my machine, and they worked in CI before the change to cfg(not.

[GuillaumeGomez]

Look at the generated docs for your test (somewhere in build/x86_64-unknown-linux-gnu/test/rustdoc-html/<your test name>/foo/) and compare with your strings.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[lcian]

Looks like f658bf6 affected this, that's why it was working locally but not in CI.

[GuillaumeGomez]

Looks all good, thanks!

Just waiting for CI to be done.

@bors squash

[rust-bors]

🔨 6 commits were squashed into 1f5a5a2.

[GuillaumeGomez]

@bors r+ rollup

[rust-bors]

📌 Commit 1f5a5a2 has been approved by GuillaumeGomez

It is now in the queue for this repository.