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

Regression in rustdoc of re-export involving namespace overlap #105735

Closed
dtolnay opened this issue Dec 15, 2022 · 2 comments · Fixed by #113785
Closed

Regression in rustdoc of re-export involving namespace overlap #105735

dtolnay opened this issue Dec 15, 2022 · 2 comments · Fixed by #113785
Labels
A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@dtolnay
Copy link
Member

dtolnay commented Dec 15, 2022

// src/lib.rs

mod thing {
    pub use core::sync::atomic::AtomicU8; // anything from a dependency, type namespace

    #[allow(non_upper_case_globals)]
    pub const AtomicU8: () = (); // value namespace
}

pub use crate::thing::AtomicU8;
cargo +nightly-2022-12-02 doc cargo +nightly-2022-12-03 doc
Screenshot from 2022-12-14 22-02-13 Screenshot from 2022-12-14 22-02-08

I believe the one on the left is what I would consider the correct rendering, and the one on the right is a regression.

The regression is in nightly-2022-12-03, specifically #104963 according to bisect, which makes sense. Mentioning @petrochenkov @cjgillot in case an obvious cause stands out to either of you.

@dtolnay dtolnay added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Dec 15, 2022
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Dec 15, 2022
@dtolnay dtolnay added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate labels Dec 15, 2022
@petrochenkov
Copy link
Contributor

in case an obvious cause stands out to either of you.

In short, rustdoc middle-end is not written well, nothing is obvious in it, and changes that are easy and predictable in rustc may lead to entirely unpredictable consequences in rustdoc.
In this specific case I investigated the affected rustdoc code just enough to make the test suite pass.

@dtolnay
Copy link
Member Author

dtolnay commented Dec 15, 2022

Mentioning @GuillaumeGomez who has been recently touching many of the same files modified in #104963 (for example in #103886 and #105183).

@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Dec 21, 2022
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jul 16, 2023
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jul 18, 2023
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jul 18, 2023
…-105735-fix, r=notriddle,aDotInTheVoid

Fix invalid display of inlined re-export when both local and foreign items are inlined

Fixes rust-lang#105735.

The bug is actually quite interesting: at the `clean` pass, local inlined items have their `use` item removed, however foreign items don't have their `use` item removed because it's in the `clean` pass that we handle them. So when a `use` inlines both a local and a foreign item, it will work as expected for the foreign one, but not for the local as its `use` should not be around anymore.

To prevent this, I created a new `inlined_foreigns` field into the `Module` struct to allow to remove the `use` item early on for foreign items as well. Then we iterate it in the `clean` pass directly.

r? `@notriddle`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jul 18, 2023
…-105735-fix, r=notriddle,aDotInTheVoid

Fix invalid display of inlined re-export when both local and foreign items are inlined

Fixes rust-lang#105735.

The bug is actually quite interesting: at the `clean` pass, local inlined items have their `use` item removed, however foreign items don't have their `use` item removed because it's in the `clean` pass that we handle them. So when a `use` inlines both a local and a foreign item, it will work as expected for the foreign one, but not for the local as its `use` should not be around anymore.

To prevent this, I created a new `inlined_foreigns` field into the `Module` struct to allow to remove the `use` item early on for foreign items as well. Then we iterate it in the `clean` pass directly.

r? ``@notriddle``
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Jul 19, 2023
…-105735-fix, r=notriddle,aDotInTheVoid

Fix invalid display of inlined re-export when both local and foreign items are inlined

Fixes rust-lang#105735.

The bug is actually quite interesting: at the `clean` pass, local inlined items have their `use` item removed, however foreign items don't have their `use` item removed because it's in the `clean` pass that we handle them. So when a `use` inlines both a local and a foreign item, it will work as expected for the foreign one, but not for the local as its `use` should not be around anymore.

To prevent this, I created a new `inlined_foreigns` field into the `Module` struct to allow to remove the `use` item early on for foreign items as well. Then we iterate it in the `clean` pass directly.

r? `@notriddle`
@bors bors closed this as completed in c7c8914 Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
4 participants