rustdoc: don't process Crate::external_traits when collecting intra-doc links
#58972
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rust-highfive
added
the
S-waiting-on-review
|
Wo, nice! r=me once CI passed |
|
Whoops, i just noticed this doesn't totally fix the linked issue. We can merge this on its own, but i'll keep digging to see what i can do about the last few warnings from rand. |
|
We can also merge mine. I prefer to wait until you have the full thing. ;) |
|
I'm hesitant to merge your PR. Muting all resolution errors from non-local items would hide the warnings that were not actually spurious (see the issue linked in the PR description for a couple examples). I think properly solving all the non-spurious warnings would require adding a much larger mechanism, or fixing |
|
I still think this is mergeable as-is; the "full thing" would add too much on top of it, since the remaining warnings in |
Centril
added
S-waiting-on-review
|
Let's merge it then, sorry for the time it took! @bors: r+ |
|
📌 Commit 49cde40 has been approved by |
bors
added
S-waiting-on-bors
Centril
added a commit
to Centril/rust
that referenced
this pull request
Apr 8, 2019
…rts, r=GuillaumeGomez rustdoc: don't process `Crate::external_traits` when collecting intra-doc links Part of rust-lang#58745, closes rust-lang#58917 The `collect-intra-doc-links` pass keeps track of the modules it recurses through as it processes items. This is used to know what module to give the resolver when looking up links. When looking through the regular items of the crate, this works fine, but the `DocFolder` trait as written doesn't just process the main crate hierarchy - it also processes the trait items in the `external_traits` map. This is useful for other passes (so they can strip out `#[doc(hidden)]` items, for example), but here it creates a situation where we're processing items "outside" the regular module hierarchy. Since everything in `external_traits` is defined outside the current crate, we can't fall back to finding its module scope like we do with local items. Skipping this collection saves us from emitting some spurious warnings. We don't even lose anything by skipping it, either - the docs loaded from here are only ever rendered through `html::render::document_short` which strips any links out, so the fact that the links haven't been loaded doesn't matter. Hopefully this removes most of the remaining spurious resolution warnings from intra-doc links. r? @GuillaumeGomez
bors
added a commit
that referenced
this pull request
Apr 9, 2019
…laumeGomez rustdoc: don't process `Crate::external_traits` when collecting intra-doc links Part of #58745, closes #58917 The `collect-intra-doc-links` pass keeps track of the modules it recurses through as it processes items. This is used to know what module to give the resolver when looking up links. When looking through the regular items of the crate, this works fine, but the `DocFolder` trait as written doesn't just process the main crate hierarchy - it also processes the trait items in the `external_traits` map. This is useful for other passes (so they can strip out `#[doc(hidden)]` items, for example), but here it creates a situation where we're processing items "outside" the regular module hierarchy. Since everything in `external_traits` is defined outside the current crate, we can't fall back to finding its module scope like we do with local items. Skipping this collection saves us from emitting some spurious warnings. We don't even lose anything by skipping it, either - the docs loaded from here are only ever rendered through `html::render::document_short` which strips any links out, so the fact that the links haven't been loaded doesn't matter. Hopefully this removes most of the remaining spurious resolution warnings from intra-doc links. r? @GuillaumeGomez
|
💔 Test failed - checks-travis |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
bors
added
S-waiting-on-review
|
@bors: retry |
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Apr 9, 2019
…laumeGomez rustdoc: don't process `Crate::external_traits` when collecting intra-doc links Part of #58745, closes #58917 The `collect-intra-doc-links` pass keeps track of the modules it recurses through as it processes items. This is used to know what module to give the resolver when looking up links. When looking through the regular items of the crate, this works fine, but the `DocFolder` trait as written doesn't just process the main crate hierarchy - it also processes the trait items in the `external_traits` map. This is useful for other passes (so they can strip out `#[doc(hidden)]` items, for example), but here it creates a situation where we're processing items "outside" the regular module hierarchy. Since everything in `external_traits` is defined outside the current crate, we can't fall back to finding its module scope like we do with local items. Skipping this collection saves us from emitting some spurious warnings. We don't even lose anything by skipping it, either - the docs loaded from here are only ever rendered through `html::render::document_short` which strips any links out, so the fact that the links haven't been loaded doesn't matter. Hopefully this removes most of the remaining spurious resolution warnings from intra-doc links. r? @GuillaumeGomez
Closed
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
💔 Test failed - checks-travis |
bors
added
S-waiting-on-review
|
@bors retry |
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Apr 11, 2019
…laumeGomez rustdoc: don't process `Crate::external_traits` when collecting intra-doc links Part of #58745, closes #58917 The `collect-intra-doc-links` pass keeps track of the modules it recurses through as it processes items. This is used to know what module to give the resolver when looking up links. When looking through the regular items of the crate, this works fine, but the `DocFolder` trait as written doesn't just process the main crate hierarchy - it also processes the trait items in the `external_traits` map. This is useful for other passes (so they can strip out `#[doc(hidden)]` items, for example), but here it creates a situation where we're processing items "outside" the regular module hierarchy. Since everything in `external_traits` is defined outside the current crate, we can't fall back to finding its module scope like we do with local items. Skipping this collection saves us from emitting some spurious warnings. We don't even lose anything by skipping it, either - the docs loaded from here are only ever rendered through `html::render::document_short` which strips any links out, so the fact that the links haven't been loaded doesn't matter. Hopefully this removes most of the remaining spurious resolution warnings from intra-doc links. r? @GuillaumeGomez
|
☀️ Test successful - checks-travis, status-appveyor |
jyn514
added
A-intra-doc-links
jyn514
added a commit
to jyn514/rust
that referenced
this pull request
Aug 29, 2020
rust-lang#58972 ignored extern_traits because before rust-lang#65983 was fixed, they would always fail to resolve, giving spurious warnings. This undoes that change, so extern traits are now seen by the `collect_intra_doc_links` pass. There are also some minor changes in librustdoc/fold.rs to avoid borrowing the extern_traits RefCell more than once at a time.
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 30, 2020
…trochenkov Fix intra-doc links for cross-crate re-exports of default trait methods The original fix for this was very simple: rust-lang#58972 ignored `extern_traits` because before rust-lang#65983 was fixed, they would always fail to resolve, giving spurious warnings. So the first commit just undoes that change, so extern traits are now seen by the `collect_intra_doc_links` pass. There are also some minor changes in `librustdoc/fold.rs` to avoid borrowing the `extern_traits` RefCell more than once at a time. However, that brought up a much more thorny problem. `rustc_resolve` started giving 'error: cannot find a built-in macro with name `cfg`' when documenting `libproc_macro` (I still haven't been able to reproduce on anything smaller than the full standard library). The chain of events looked like this (thanks @eddyb for the help debugging!): 0. `x.py build --stage 1` builds the standard library and creates a sysroot 1. `cargo doc` does something like `cargo check` to create `rmeta`s for all the crates (unrelated to what was built above) 2. the `cargo check`-like `libcore-*.rmeta` is loaded as a transitive dependency *and claims ownership* of builtin macros 3. `rustdoc` later tries to resolve some path in a doc link 4. suggestion logic fires and loads "extern prelude" crates by name 5. the sysroot `libcore-*.rlib` is loaded and *fails to claim ownership* of builtin macros `rustc_resolve` gives the error after step 5. However, `rustdoc` doesn't need suggestions at all - `resolve_str_path_error` completely discards the `ResolutionError`! The fix implemented in this PR is to skip the suggestion logic for `resolve_ast_path`: pass `record_used: false` and skip `lookup_import_candidates` when `record_used` isn't set. It's possible that if/when rust-lang#74207 is implemented this will need a more in-depth fix which returns a `ResolutionError` from `compile_macro`, to allow rustdoc to reuse the suggestions from rustc_resolve. However, that's a much larger change and there's no need for it yet, so I haven't implemented it here. Fixes rust-lang#73829. r? @GuillaumeGomez
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of #58745, closes #58917
The
collect-intra-doc-linkspass keeps track of the modules it recurses through as it processes items. This is used to know what module to give the resolver when looking up links. When looking through the regular items of the crate, this works fine, but theDocFoldertrait as written doesn't just process the main crate hierarchy - it also processes the trait items in theexternal_traitsmap. This is useful for other passes (so they can strip out#[doc(hidden)]items, for example), but here it creates a situation where we're processing items "outside" the regular module hierarchy. Since everything inexternal_traitsis defined outside the current crate, we can't fall back to finding its module scope like we do with local items.Skipping this collection saves us from emitting some spurious warnings. We don't even lose anything by skipping it, either - the docs loaded from here are only ever rendered through
html::render::document_shortwhich strips any links out, so the fact that the links haven't been loaded doesn't matter. Hopefully this removes most of the remaining spurious resolution warnings from intra-doc links.r? @GuillaumeGomez