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: crate::ptr on a re-export of core::slice is not resolved in alloc #76106

Closed
jyn514 opened this issue Aug 30, 2020 · 17 comments
Closed
Labels
A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate 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-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Aug 30, 2020

It's something to do specifically with the re-export because using ptr instead of crate::ptr works, possibly because it's available in both alloc::slice and core::slice.

I don't understand why this is failing :/ the error is being returned directly from rustc_resolve:

$ RUSTDOC_LOG=rustdoc::passes::collect=debug,rustc_resolve=debug xpy test --stage 1 linkchecker library/alloc
: Some(NodeId(101625)), has_generic_args: false }], opt_ns=Some(MacroNS), record_used=true, path_span=library/alloc/src/lib.rs:1:1: 1:1 (#0), crate_lint=No)
2020-08-29T22:46:53.052491Z DEBUG rustc_resolve: resolve_path ident 0 crate#0 Some(NodeId(101624))
2020-08-29T22:46:53.052494Z DEBUG rustc_resolve: resolve_crate_root(crate#0)
2020-08-29T22:46:53.052496Z DEBUG rustc_resolve: resolve_crate_root: not DollarCrate
2020-08-29T22:46:53.052499Z DEBUG rustc_resolve: resolve_crate_root(crate#0): found no mark (ident.span = library/alloc/src/lib.rs:1:1: 1:1 (#0))
2020-08-29T22:46:53.052502Z DEBUG rustc_resolve: resolve_path ident 1 ptr#0 Some(NodeId(101625))
2020-08-29T22:46:53.053654Z DEBUG rustc_resolve: resolve_path(path=[Segment { ident: crate#0, id: Some(NodeId(101626)), has_generic_args: false }, Segment { ident: ptr#0, id
: Some(NodeId(101627)), has_generic_args: false }], opt_ns=Some(TypeNS), record_used=true, path_span=library/alloc/src/lib.rs:1:1: 1:1 (#0), crate_lint=No)
2020-08-29T22:46:53.053666Z DEBUG rustc_resolve: resolve_path ident 0 crate#0 Some(NodeId(101626))
2020-08-29T22:46:53.053669Z DEBUG rustc_resolve: resolve_crate_root(crate#0)
2020-08-29T22:46:53.053671Z DEBUG rustc_resolve: resolve_crate_root: not DollarCrate
2020-08-29T22:46:53.053673Z DEBUG rustc_resolve: resolve_crate_root(crate#0): found no mark (ident.span = library/alloc/src/lib.rs:1:1: 1:1 (#0))
2020-08-29T22:46:53.053677Z DEBUG rustc_resolve: resolve_path ident 1 ptr#0 Some(NodeId(101627))
2020-08-29T22:46:53.054499Z DEBUG rustdoc::passes::collect_intra_doc_links: crate::ptr resolved to Err(()) in namespace TypeNS with parent DefId(1:6606 ~ core[c85e]::slice[0
])
2020-08-29T22:46:53.055390Z  INFO rustdoc::passes::collect_intra_doc_links: ignoring warning from parent crate: unresolved link to `crate::ptr`

In fact if you look at the debug logging it says itself that it's looking at core: parent DefId(1:6606 ~ core[c85e]::slice[0]). Not sure what's going wrong.

It would be great to find an MCVE of this.

Originally posted by @jyn514 in #75932 (comment)

@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 E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example labels Aug 30, 2020
@jyn514
Copy link
Member Author

jyn514 commented Aug 30, 2020

Direct link to the failing intra-doc link: 6aae4a2#diff-e5a4dbd27d5eadb7a3cf746c396f5420R6430

@jyn514 jyn514 added the A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate label Aug 30, 2020
@jyn514
Copy link
Member Author

jyn514 commented Sep 2, 2020

Some more test cases from #76235:

std/sync/struct.Arc.html:214: broken link - std/sync/mem::MaybeUninit::method.assume_init
std/sync/struct.Arc.html:238: broken link - std/sync/mem::MaybeUninit::method.assume_init
alloc/sync/struct.Arc.html:214: broken link - alloc/sync/mem::MaybeUninit::method.assume_init
alloc/sync/struct.Arc.html:238: broken link - alloc/sync/mem::MaybeUninit::method.assume_init
thread 'main' panicked at 'found some broken links', src/tools/linkchecker/main.rs:67:9

@bors bors closed this as completed in 44bacc3 Sep 3, 2020
@jyn514
Copy link
Member Author

jyn514 commented Sep 3, 2020

GitHub was overeager, this is not fixed, just worked around.

@jyn514 jyn514 reopened this Sep 3, 2020
dylni added a commit to dylni/quit that referenced this issue Sep 25, 2020
@dylni
Copy link
Contributor

dylni commented Sep 25, 2020

@jyn514 I had this issue when documenting quit. It might be easier to test with that crate, since it's small and not part of libstd, but the issue is a little different. crate::with_code works, but with_code is unresolved.

Rustdoc gives no warnings with either.

@jyn514
Copy link
Member Author

jyn514 commented Sep 25, 2020

@dylni that's a separate issue - with_code is additional documentation on the re-export that gets resolved in the scope of the original module instead of in the scope of the re-export. This issue is about documentation that was present to begin with and mysteriously breaks.

@jyn514
Copy link
Member Author

jyn514 commented Sep 25, 2020

The fact that it works with crate:: is really odd though. Sounds like something weird is going on with proc-macros, can you open a new issue?

@jyn514
Copy link
Member Author

jyn514 commented Sep 25, 2020

Also, I'm starting to think we should revert #56922, it's causing more trouble than it's worth.

@dylni
Copy link
Contributor

dylni commented Sep 25, 2020

Sure: #77193

@dylni
Copy link
Contributor

dylni commented Sep 25, 2020

Also, I'm starting to think we should revert #56922, it's causing more trouble than it's worth.

That explains why there aren't any warnings. I agree it might be better to revert that fix. If I'm running with --display-warnings, I would want to see anything that will show as broken in the documentation.

P.S. Is it expected that [`[String]`] doesn't cause a warning and doesn't resolve?

@jyn514
Copy link
Member Author

jyn514 commented Sep 25, 2020

It's definitely expected that it doesn't resolve: You said 'a link to `[String]`', which gets the backticks stripped and rustdoc then sees [String], which isn't a valid rust path.

[] not warning is intentional but possibly not the best decision. The code for that is here:

if path_str.contains(|ch: char| !(ch.is_alphanumeric() || ch == ':' || ch == '_')) {
return;
}

@dylni
Copy link
Contributor

dylni commented Sep 25, 2020

rustdoc then sees [String], which isn't a valid rust path.

My intention was to link to the slice primitive (i.e., &[String]). I know now that intra-rustdoc links don't work on types that include their generic parameters, but it might be an easy thing to miss without the warning. Should I create an issue/PR for adding a warning there?

Also, sorry for hijacking this issue.

@jyn514
Copy link
Member Author

jyn514 commented Sep 25, 2020

Sure, go ahead and open an issue for that too.

Do you mind joining the discord so we can talk about this more without hijacking the issue further? https://discord.gg/4yEYPuT

@dylni
Copy link
Contributor

dylni commented Sep 25, 2020

Discord is refusing to load messages :(

@jyn514
Copy link
Member Author

jyn514 commented Sep 25, 2020

Closing as duplicate of #76106, which has more discussion.

@jyn514 jyn514 closed this as completed Sep 25, 2020
@dylni
Copy link
Contributor

dylni commented Sep 25, 2020

@jyn514 I think you meant #77193. ;)

@camelid
Copy link
Member

camelid commented Oct 12, 2020

I know now that intra-rustdoc links don't work on types that include their generic parameters

They do now! #76934 :)

@dylni
Copy link
Contributor

dylni commented Oct 12, 2020

@camelid

They do now! #76934 :)

Thanks! Unfortunately, [u8] is still unresolved, but that doesn't technically use generics. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate 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-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example 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