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

Intra-doc links do not resolve associated items for blanket impls #78800

Open
jyn514 opened this issue Nov 6, 2020 · 6 comments
Open

Intra-doc links do not resolve associated items for blanket impls #78800

jyn514 opened this issue Nov 6, 2020 · 6 comments
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name A-traits Area: Trait system C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Nov 6, 2020

I tried this code:

pub trait Trait {
    fn f();
}
impl<T> Trait for T {
    fn f() {}
}

pub mod inner {
    use crate::Trait;
    /// Link to [S::f]
    pub struct S;
}

I expected to see this happen: The link resolves to <S as Trait>::f

Instead, this happened: rustdoc can't resolve the link:

warning: unresolved link to `S::f`
  --> blanket.rs:10:18
   |
10 |     /// Link to [S::f]
   |                  ^^^^ the struct `S` has no field or associated item named `f`
   |
   = note: `#[warn(broken_intra_doc_links)]` on by default

Meta

rustdoc +nightly --version: rustdoc 1.49.0-nightly (ffa2e7ae8 2020-10-24)

cc @seeplusplus

After seeing #78761 I'm strongly considering getting rid of blanket impls altogether, clearly no one has been using them or they would have opened an issue. @Manishearth what do you think?

@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-bug Category: This is a bug. A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name labels Nov 6, 2020
@jyn514
Copy link
Member Author

jyn514 commented Nov 6, 2020

Note that rustc resolves it fine if you call inner::S::f(); in a main function: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=973602a904ba477d3edd9ec90e6729a5

@jyn514
Copy link
Member Author

jyn514 commented Nov 6, 2020

Another related issue for after this is fixed: right now rustdoc doesn't consider scoping at all for blanket impls, only for direct impls like impl Trait for MyType. Notice how module isn't passed to get_blanket_impls:

let implicit_impls = crate::clean::get_auto_trait_and_blanket_impls(cx, ty, did);

@jyn514 jyn514 added the A-traits Area: Trait system label Nov 6, 2020
@jyn514
Copy link
Member Author

jyn514 commented Nov 6, 2020

rustdoc doesn't consider scoping at all for blanket impls

If we change get_blanket_impls to take a filter parameter like I suggested in #78761 (comment), we could filter traits in scope there. The way to tell that is

        cx.enter_resolver(|resolver| {
            resolver.traits_in_scope(module).iter().find(|candidate| candidate.def_id == trait_).is_some()
        })

which is not as efficient as only looking up the relevant traits in the first place but I think is the best we can do while still being able to reuse the code for clean.

@seeplusplus
Copy link

Currently looking into fixing this,
@rustbot claim

bors added a commit to rust-lang-ci/rust that referenced this issue Dec 16, 2020
…h,GuillaumeGomez

Don't look for blanket impls in intra-doc links

This never worked and has been causing severe performance problems.
Hopefully it will be re-landed at some point in the future when it
actually works, but in the meantime it makes no sense to have the code
around when it does nothing and actively makes rustdoc harder to use.

Closes rust-lang#78761. Does *not* affect rust-lang#78800.

r? `@Manishearth`
cc `@seeplusplus`
JohnTitor added a commit to JohnTitor/rust that referenced this issue Mar 2, 2021
Update intra-doc link documentation to match the implementation

r? `@Manishearth`
cc `@camelid` `@m-ou-se`

Relevant PRs:
- rust-lang#74489
- rust-lang#80181
- rust-lang#76078
- rust-lang#77519
- rust-lang#73101

Relevant issues:
- rust-lang#78800
- rust-lang#77200
- rust-lang#77199 / rust-lang#54191

I haven't documented things that I consider 'just bugs', like rust-lang#77732, but I have documented features that aren't implemented, like rust-lang#78800.
@jyn514
Copy link
Member Author

jyn514 commented Jun 5, 2021

@seeplusplus hi, are you still working on this? It's ok if not, but please comment @rustbot release-assignment if so.

@seeplusplus
Copy link

@rustbot release-assignment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name A-traits Area: Trait system C-bug Category: This is a bug. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants