Skip to content

Rustdoc doesn't warn if links to associated items are ambiguous if primitives are involved #83862

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

Closed
jyn514 opened this issue Apr 4, 2021 · 5 comments
Labels
A-associated-items Area: Associated items (types, constants & functions) A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name 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 Apr 4, 2021

Rustdoc should give an error if the base res is ambiguous, but it doesn't:

$ cat primitive-ambiguous.rs 
//! [usize::MAX]

mod usize {
  const MAX: usize = 1;
}
$ rustdoc primitive-ambiguous.rs
$

This was originally discovered through the special case

//! [usize::MAX]

#[doc(primitive = "usize")]
mod usize {}

which I think should continue working, rustdoc shouldn't give an ambiguity error if the module has doc(primitive) on it.

Originally posted by @jyn514 in #83849 (comment)

Meta

HEAD is 2616ab1

@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-associated-items Area: Associated items (types, constants & functions) A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name labels Apr 4, 2021
@jyn514 jyn514 added the C-bug Category: This is a bug. label Apr 5, 2021
@jyn514
Copy link
Member Author

jyn514 commented Apr 8, 2021

Oof I just realized this is a lot harder than I expected. For the case where usize is a module, usize::MAX resolves normally with resolve_path, before the hacky string splitting for resolve_associated_item. So to support prim@usize::MAX I need to do the string splitting before resolve_path ever runs, and call resolve_primitive_associated_item instead.

@Manishearth btw do you want that prim@usize::MAX syntax to work? Or should it just not be possible to disambiguate the two? It does seem weird for the disambiguator to refer to the root res usize instead of MAX, that's inconsistent with how it works eveywhere else.

@Manishearth
Copy link
Member

@Manishearth btw do you want that prim@usize::MAX syntax to work? Or should it just not be possible to disambiguate the two? It does seem weird for the disambiguator to refer to the root res usize instead of MAX, that's inconsistent with how it works eveywhere else.

Hmm, this sounds like a pretty big complication; unsure if we should.

We can just special case the primitives and do both lookups.

@jyn514
Copy link
Member Author

jyn514 commented Apr 8, 2021

We can just special case the primitives and do both lookups.

Sorry, I don't follow - what should happen if it's ambiguous (both resolve)?

@jyn514 jyn514 changed the title Rustdoc doesn't warn if links to associated items are ambiguous Rustdoc doesn't warn if links to associated items are ambiguous if primitives are involved Apr 8, 2021
@jyn514
Copy link
Member Author

jyn514 commented Apr 8, 2021

I would also be ok with saying "don't name modules after primitives" and closing this as WONTFIX, that seems confusing anyway.

@Manishearth
Copy link
Member

I mean, don't name modules after primitives such that the names clash. Fine with wontfixing

@jyn514 jyn514 closed this as completed Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-associated-items Area: Associated items (types, constants & functions) A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name 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

2 participants