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

Rustdoc doesn't see trait implementations inside of functions #41480

Closed
RReverser opened this issue Apr 23, 2017 · 8 comments · Fixed by #53162
Closed

Rustdoc doesn't see trait implementations inside of functions #41480

RReverser opened this issue Apr 23, 2017 · 8 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

@RReverser
Copy link
Contributor

It's possible to define trait implementations inside of functions in Rust. This is useful not too often, but particularly nice if you have a macro that both defines trait implementation and calls some initialization for a given type.

Anyway, the following code is valid in Rust itself:

pub struct S;

pub fn a() {
    impl From<u8> for S {
        fn from(_: u8) -> S {
            S
        }
    }
}

But Rustdoc doesn't generate documentation for a given trait, despite it being visible to Rust itself, and being immediately available and unique across entire crate.

I can move impl out of the function and then it gets seen by Rustdoc, but this is rather inconvenient when impl is generated as part of a bigger macro.

@GuillaumeGomez
Copy link
Member

The scope of the implementation is only inside the function and so cannot be used outside of it and so shouldn't be documented. Also, macro != functions. If you make an implementation inside a macro, it's documented.

Closing it then, don't hesitate to reopen it if I missed something.

@RReverser
Copy link
Contributor Author

RReverser commented Apr 25, 2017

The scope of the implementation is only inside the function and so cannot be used outside of it and so shouldn't be documented.

No, this is exactly the problem I was referring to - impl is not scoped. In Rust itself it's visible across entire codebase despite being declared inside of a function.

Also, macro != functions. If you make an implementation inside a macro, it's documented.

I referred to macro only because I'm using them inside of such function to generate both impl and a call to some other function.

@RReverser
Copy link
Contributor Author

RReverser commented Apr 25, 2017

To enhance the example above:

pub struct S;

pub fn a() {
    impl From<u8> for S {
        fn from(_: u8) -> S {
            S
        }
    }
}

fn main() {
    let s = S::from(10u8);
}

^^ From works here as expected, as well as other crates that depend on this one, can use it, which is why it's important to have it documented.

@GuillaumeGomez

@petrochenkov
Copy link
Contributor

don't hesitate to reopen it if I missed something

(GitHub doesn't allow non-members to reopen issues closed by others.)

@petrochenkov petrochenkov reopened this Apr 25, 2017
@RReverser
Copy link
Contributor Author

doesn't allow non-members to reopen issues closed by others

Indeed, thanks for reopening this!

@GuillaumeGomez
Copy link
Member

Thanks for reopening it @petrochenkov.

@RReverser: That's completely crazy. Another example just for the fun:

pub struct S;

trait Foo {
    fn foo(&self) {}
}

pub fn a() {
    impl Foo for S {
    }
}

fn main() {
    S.foo();
}

So rustdoc should document these implementations as well. Thanks for opening the issue!

@GuillaumeGomez GuillaumeGomez added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools labels Apr 25, 2017
@frewsxcv frewsxcv added T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. and removed A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools labels May 18, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 27, 2017
@Marwes
Copy link
Contributor

Marwes commented Nov 29, 2017

Got bit by this. rustdoc were simply unable to find a trait implementation defined in a function which cargo build picked up fine https://docs.rs/crate/serde_state/0.4.2/builds/77728 . Thought I was going crazy for second there.

@QuietMisdreavus
Copy link
Member

I've opened #53162 to fix this.

bors added a commit that referenced this issue Sep 7, 2018
…=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
…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
…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
…=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
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
Development

Successfully merging a pull request may close this issue.

7 participants