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

make everybody_loops preserve item declarations #53002

Merged

Conversation

QuietMisdreavus
Copy link
Member

First half of #52545.

everybody_loops is used by rustdoc to ensure we don't contain erroneous references to platform APIs if one of its uses is pulled in by #[doc(cfg)]. However, you can also implement traits for public types inside of functions. This is used by Diesel (probably others, but they were the example that was reported) to get around a recent macro hygiene fix, which has caused their crate to fail to document. While this won't make the traits show up in documentation (that step comes later), it will at least allow files to be generated.

@rust-highfive
Copy link
Collaborator

r? @varkor

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 2, 2018
@QuietMisdreavus
Copy link
Member Author

r? @pnkfelix @eddyb

@rust-highfive rust-highfive assigned pnkfelix and unassigned varkor Aug 2, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Aug 3, 2018

I'll make sure to make a decision about this tomorrow.

@pnkfelix
Copy link
Member

pnkfelix commented Aug 3, 2018

Thanks for the fun looking at the design options here via live chat over last few days with @QuietMisdreavus and @eddyb

I have an alternative approach that is factored slightly differently, but until I see a case where this PR breaks, I'm not going to try to push it.

@pnkfelix
Copy link
Member

pnkfelix commented Aug 3, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Aug 3, 2018

📌 Commit 7e77d19 has been approved by pnkfelix

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 3, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Aug 3, 2018

For the record, below (in the details block) is an expanded test case that tries to trip up code here. It might be worth trying to adapt into a real test for rustdoc at some point.

  • The goal of the tests I make, in terms of how raw -Z unpretty=everybody_loops is supposed to behave, is that for any test that rustc compiles without error, you should be able to roundtrip the output of -Z unpretty=everybody_loops back into rustc again.
  • From what I can see, this PR satisfies that goal for the test below
#![allow(dead_code)]

mod a { fn f() { println!("Hello from a::f"); } }

trait Trait {
    fn m1(&self);
    fn m2(&self) { }
}

struct S1;
struct S2;

mod b {
    fn f() { println!("Hello from b::f"); }
    struct S3;
    impl ::Trait for S3 {
        fn m1(&self) { println!("Hello from S3::m1"); }
        fn m2(&self) { println!("Hello from S3::m2"); }
    }
}

struct S4;

struct S6;

fn main () {
    mod c { fn f() { println!("Hello from c::f"); } }

    const C: [i32; 2] = [1, 2];

    { mod c2 { const C: [i32; 2] = [1, 2]; } };

    { mod c2 { const C: [i32; 2] = [1, 2]; } }

    { mod c2 { const C: [i32; { mod uh_oh {  } 2 }] = [1, 2]; } }

    { mod c2 { const C: [i32; { mod uh_oh { impl ::Trait for ::S6 { fn m1(&self) { } } } 2 }] = [1, 2]; } }

    struct S5;

    mod d {
        trait Trait { fn m1(&self); }
        impl Trait for ::S4 {
            fn m1(&self) { println!("Hello from <S4 as d::Trait>::m1"); }
        }
    }

    impl Trait for S4 {
        fn m1(&self) { println!("Hello from <S4 as Trait>::m1"); }
    }

    impl Trait for S5 {
        fn m1(&self) { println!("Hello from <S4 as Trait>::m1"); }
    }

    impl S1 {
        fn f() { println!("Hello from S1::f"); }
    }

    impl Trait for S1 {
        fn m1(&self) { println!("Hello from S1::m1"); }
        fn m2(&self) { println!("Hello from S1::m2"); }
    }

    mod e {
        fn f() {
            impl ::Trait for ::S2 {
                fn m1(&self) { println!("Hello from S2::m1"); }
                fn m2(&self) { println!("Hello from S2::m2"); }
            }
        }
    }

    println!("Hello from main");
}

cramertj added a commit to cramertj/rust that referenced this pull request Aug 3, 2018
…some-loops, r=pnkfelix

make `everybody_loops` preserve item declarations

First half of rust-lang#52545.

`everybody_loops` is used by rustdoc to ensure we don't contain erroneous references to platform APIs if one of its uses is pulled in by `#[doc(cfg)]`. However, you can also implement traits for public types inside of functions. This is used by Diesel (probably others, but they were the example that was reported) to get around a recent macro hygiene fix, which has caused their crate to fail to document. While this won't make the traits show up in documentation (that step comes later), it will at least allow files to be generated.
@bors
Copy link
Contributor

bors commented Aug 6, 2018

⌛ Testing commit 7e77d19 with merge aa1e6db...

bors added a commit that referenced this pull request Aug 6, 2018
… r=pnkfelix

make `everybody_loops` preserve item declarations

First half of #52545.

`everybody_loops` is used by rustdoc to ensure we don't contain erroneous references to platform APIs if one of its uses is pulled in by `#[doc(cfg)]`. However, you can also implement traits for public types inside of functions. This is used by Diesel (probably others, but they were the example that was reported) to get around a recent macro hygiene fix, which has caused their crate to fail to document. While this won't make the traits show up in documentation (that step comes later), it will at least allow files to be generated.
@bors
Copy link
Contributor

bors commented Aug 6, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: pnkfelix
Pushing aa1e6db to master...

@bors bors merged commit 7e77d19 into rust-lang:master Aug 6, 2018
@QuietMisdreavus QuietMisdreavus deleted the brother-may-i-have-some-loops branch August 7, 2018 15:30
bors added a commit that referenced this pull request 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 pull request 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 pull request 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 pull request 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!
@jyn514 jyn514 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Sep 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants