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.

Sign up for GitHub

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

Jump to bottom

rustdoc: Trait implementations aren't listed in generated docs under some circumstances #36922

Closed
jimmycuadra opened this issue Oct 3, 2016 · 5 comments · Fixed by #53162
Closed

rustdoc: Trait implementations aren't listed in generated docs under some circumstances #36922

jimmycuadra opened this issue Oct 3, 2016 · 5 comments · Fixed by #53162
Labels
C-bug Category: This is a bug. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@jimmycuadra
Copy link
Contributor

Serde's Deserialize and Serialize traits don't show up in the "Trait Implementations" section for types that use custom derive through Macros 1.1 or through Syntex. Alex Crichton also notes that rustdoc does not display Copy in the "Trait Implementations" section with the following code:

#[derive(Clone)]           
pub struct Point {         
    x: i32,                
    y: i32,                
}                          

const _FOO: () = {         
    impl Copy for Point {} 
    ()                     
};

I've only tested this on nightly Rust.
@eddyb
Copy link
Member

eddyb commented Oct 3, 2016

@alexcrichton Ugh, the way rustdoc scans a crate is dubious at best. I've been having to patch up rustdoc for various typesystem/metadata changes, and it's really sad how items from extern crates are second-class, but at the same time you have bugs like these because the local crate visit is simplistic.

What's needed is a rustdoc rewrite that does as little as possible, working mostly with non-syntactical information and recovering the minimum required - relying too much on syntax can leak confusing context.
And it really shouldn't have two ASTs of its own. I know @cmr was frustrated with libsyntax but rustdoc is barely maintainable right now.

@apasel422 apasel422 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Oct 3, 2016
@alexcrichton
Copy link
Member

@eddyb I think rustdoc's slogan is "dubious at best"

@TyOverby
Copy link
Contributor

This bug is particularly confusing for crates that use Serde. Right now auto-derived implementations of Serialize/Deserialize don't show up in rustdoc.

@steveklabnik steveklabnik added T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. and removed T-tools labels May 18, 2017
danburkert added a commit to tokio-rs/prost that referenced this issue Jul 11, 2017
@danburkert
Use 'dummy module' instead of 'dummy const' in derive output
446f0ce 
The 'dummy const' pattern was copied from SerDe[1], but it causes
rustdoc not to generate documentation due to a [known bug][2].
Apparently SerDe [considered][3] using a dummy module instead of a dummy
const, but decided against it due to private type issues. Since
everything's public in `prost`, we shouldn't have such an issue.

Also removes #[automatically_derived] annotations, since apparently the
no longer do anything[4].

Fixes #11

[1]: https://github.com/serde-rs/serde/blob/775e8154e7151eb1576d65df539c4ac1612595c6/serde_derive/src/ser.rs#L28
[2]: rust-lang/rust#36922
[3]: serde-rs/serde#159 (comment)
[4]: https://botbot.me/mozilla/rust/2017-07-11/?msg=88402326&page=3
danburkert added a commit to tokio-rs/prost that referenced this issue Jul 11, 2017
@danburkert
Use 'dummy module' instead of 'dummy const' in derive output
00bb1ca 
The 'dummy const' pattern was copied from SerDe[1], but it causes
rustdoc not to generate documentation due to a [known bug][2].
Apparently SerDe [considered][3] using a dummy module instead of a dummy
const, but decided against it due to private type issues. Since
everything's public in `prost`, we shouldn't have such an issue.

Also removes #[automatically_derived] annotations, since apparently the
no longer do anything[4].

Fixes #11

[1]: https://github.com/serde-rs/serde/blob/775e8154e7151eb1576d65df539c4ac1612595c6/serde_derive/src/ser.rs#L28
[2]: rust-lang/rust#36922
[3]: serde-rs/serde#159 (comment)
[4]: https://botbot.me/mozilla/rust/2017-07-11/?msg=88402326&page=3
danburkert added a commit to tokio-rs/prost that referenced this issue Jul 11, 2017
@danburkert
Use 'dummy module' instead of 'dummy const' in derive output
0bdd878 
The 'dummy const' pattern was copied from SerDe[1], but it causes
rustdoc not to generate documentation due to a [known bug][2].
Apparently SerDe [considered][3] using a dummy module instead of a dummy
const, but decided against it due to private type issues. Since
everything's public in `prost`, we shouldn't have such an issue.

Also removes #[automatically_derived] annotations, since apparently the
no longer do anything[4].

Fixes #11

[1]: https://github.com/serde-rs/serde/blob/775e8154e7151eb1576d65df539c4ac1612595c6/serde_derive/src/ser.rs#L28
[2]: rust-lang/rust#36922
[3]: serde-rs/serde#159 (comment)
[4]: https://botbot.me/mozilla/rust/2017-07-11/?msg=88402326&page=3
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 26, 2017
@pitdicker pitdicker mentioned this issue Apr 2, 2018
Merged
@shepmaster
Copy link
Member

Whew! This is super confusing!

@ollie27 ollie27 mentioned this issue Jul 26, 2018
Closed
@QuietMisdreavus QuietMisdreavus mentioned this issue Aug 7, 2018
Merged
@QuietMisdreavus
Copy link
Member

I've opened #53162 to fix this.

bors added a commit that referenced this issue Sep 7, 2018
@bors
Auto merge of #53162 - QuietMisdreavus:crouching-impl-hidden-trait, r…
2708e39 
…=GuillaumeGomez

rustdoc: collect trait impls as an early pass

Fixes #52545, fixes #41480, fixes #36922

Right now, rustdoc pulls all its impl information by scanning a crate's HIR for any items it finds. However, it doesn't recurse into anything other than modules, preventing it from seeing trait impls that may be inside things like functions or consts. Thanks to #53002, now these items actually *exist* for rustdoc to see, but they still weren't getting collected for display.

But there was a secret. Whenever we pull in an item from another crate, we don't have any of its impls in the local HIR, so instead we ask the compiler for *everything* and filter out after the fact. This process is only triggered if there's a cross-crate re-export in the crate being documented, which can sometimes leave this info out of the docs. This PR instead moves this collection into an early pass, which occurs immediately after crate cleaning, so that that collection occurs regardless. In addition, by including the HIR's own `trait_impls` in addition to the existing `all_trait_implementations` calls, we can collect all these tricky trait impls without having to scan for them!
kennytm added a commit to kennytm/rust that referenced this issue Sep 7, 2018
@kennytm
Rollup merge of rust-lang#53162 - QuietMisdreavus:crouching-impl-hidd…
62d1e0c 
…en-trait, r=GuillaumeGomez

rustdoc: collect trait impls as an early pass

Fixes rust-lang#52545, fixes rust-lang#41480, fixes rust-lang#36922

Right now, rustdoc pulls all its impl information by scanning a crate's HIR for any items it finds. However, it doesn't recurse into anything other than modules, preventing it from seeing trait impls that may be inside things like functions or consts. Thanks to rust-lang#53002, now these items actually *exist* for rustdoc to see, but they still weren't getting collected for display.

But there was a secret. Whenever we pull in an item from another crate, we don't have any of its impls in the local HIR, so instead we ask the compiler for *everything* and filter out after the fact. This process is only triggered if there's a cross-crate re-export in the crate being documented, which can sometimes leave this info out of the docs. This PR instead moves this collection into an early pass, which occurs immediately after crate cleaning, so that that collection occurs regardless. In addition, by including the HIR's own `trait_impls` in addition to the existing `all_trait_implementations` calls, we can collect all these tricky trait impls without having to scan for them!
kennytm added a commit to kennytm/rust that referenced this issue Sep 8, 2018
@kennytm
Rollup merge of rust-lang#53162 - QuietMisdreavus:crouching-impl-hidd…
4b2be62 
…en-trait, r=GuillaumeGomez

rustdoc: collect trait impls as an early pass

Fixes rust-lang#52545, fixes rust-lang#41480, fixes rust-lang#36922

Right now, rustdoc pulls all its impl information by scanning a crate's HIR for any items it finds. However, it doesn't recurse into anything other than modules, preventing it from seeing trait impls that may be inside things like functions or consts. Thanks to rust-lang#53002, now these items actually *exist* for rustdoc to see, but they still weren't getting collected for display.

But there was a secret. Whenever we pull in an item from another crate, we don't have any of its impls in the local HIR, so instead we ask the compiler for *everything* and filter out after the fact. This process is only triggered if there's a cross-crate re-export in the crate being documented, which can sometimes leave this info out of the docs. This PR instead moves this collection into an early pass, which occurs immediately after crate cleaning, so that that collection occurs regardless. In addition, by including the HIR's own `trait_impls` in addition to the existing `all_trait_implementations` calls, we can collect all these tricky trait impls without having to scan for them!
bors added a commit that referenced this issue Sep 20, 2018
@bors
Auto merge of #53162 - QuietMisdreavus:crouching-impl-hidden-trait, r…
3bc2ca7 
…=GuillaumeGomez

rustdoc: collect trait impls as an early pass

Fixes #52545, fixes #41480, fixes #36922

Right now, rustdoc pulls all its impl information by scanning a crate's HIR for any items it finds. However, it doesn't recurse into anything other than modules, preventing it from seeing trait impls that may be inside things like functions or consts. Thanks to #53002, now these items actually *exist* for rustdoc to see, but they still weren't getting collected for display.

But there was a secret. Whenever we pull in an item from another crate, we don't have any of its impls in the local HIR, so instead we ask the compiler for *everything* and filter out after the fact. This process is only triggered if there's a cross-crate re-export in the crate being documented, which can sometimes leave this info out of the docs. This PR instead moves this collection into an early pass, which occurs immediately after crate cleaning, so that that collection occurs regardless. In addition, by including the HIR's own `trait_impls` in addition to the existing `all_trait_implementations` calls, we can collect all these tricky trait impls without having to scan for them!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Category: This is a bug. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

rustdoc: collect trait impls as an early pass QuietMisdreavus/rust
9 participants
@steveklabnik @alexcrichton @eddyb @jimmycuadra @shepmaster @TyOverby @Mark-Simulacrum @QuietMisdreavus @apasel422