Skip to content

Intra-doc links to items in std::primitive are broken #76692

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
ollie27 opened this issue Sep 14, 2020 · 3 comments · Fixed by #76955
Closed

Intra-doc links to items in std::primitive are broken #76692

ollie27 opened this issue Sep 14, 2020 · 3 comments · Fixed by #76955
Assignees
Labels
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. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@ollie27
Copy link
Member

ollie27 commented Sep 14, 2020

For example [`std::primitive::i32`] generates a link to https://doc.rust-lang.org/nightly/std/primitive.std::primitive::i32.html.

@ollie27 ollie27 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 Sep 14, 2020
@jyn514 jyn514 added E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Sep 14, 2020
@jyn514
Copy link
Member

jyn514 commented Sep 14, 2020

I didn't realize you could re-export primitives, good catch!

This will need a redesign of how rustdoc links primitives: right now it does pure text comparison, which will have to be changed to go through rustc_resolve. I think just looking at Res instead of is_primitive will work for almost everything, since you can't define a primitive and module in the same scope:

mod inner {
    pub use u64;
    //^~ ERROR ambiguous
    
    pub mod u64 {}
    //^~ ERROR defined multiple times
}

The only thing rustdoc needs to second-guess resolve on is for primitives and modules with the same name:

/// [prim@char]
/// [mod@char]
mod char {}

So this usage needs to stay:

if let Some((path, prim)) = is_primitive(path_str, TypeNS) {

but everything else should be removed and check Res::PrimTy instead.

@jyn514
Copy link
Member

jyn514 commented Sep 14, 2020

I expect fixing this to also fix #76693 at the same time.

@jyn514
Copy link
Member

jyn514 commented Sep 19, 2020

A related issue: when this is fixed, the link text displayed for primitives will be wrong:

link_text: path_str.to_owned(),

That should just say link_text instead.

@jyn514 jyn514 self-assigned this Sep 20, 2020
RalfJung added a commit to RalfJung/rust that referenced this issue Sep 25, 2020
Refactor and fix intra-doc link diagnostics, and fix links to primitives

Closes rust-lang#76925, closes rust-lang#76693, closes rust-lang#76692.

Originally I only meant to fix rust-lang#76925. But the hack with `has_primitive` was so bad it was easier to fix the primitive issues than to try and work around it.

Note that this still has one bug: `std::primitive::i32::MAX` does not resolve. However, this fixes the ICE so I'm fine with fixing the link in a later PR.

This is part of a series of refactors to make rust-lang#76467 possible.

This is best reviewed commit-by-commit; it has detailed commit messages.

r? @euclio
@bors bors closed this as completed in 71bdb84 Sep 27, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
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. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. 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.

2 participants