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

Remove intra-docs check for items from dependencies #58917

Closed

Conversation

GuillaumeGomez
Copy link
Member

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 4, 2019
@rust-highfive

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Mar 10, 2019

☔ The latest upstream changes (presumably #56732) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Mar 18, 2019

☔ The latest upstream changes (presumably #58824) made this pull request unmergeable. Please resolve the merge conflicts.

@QuietMisdreavus
Copy link
Member

See #58972 for discussion about this PR.

@Centril
Copy link
Contributor

Centril commented Mar 30, 2019

Ping from triage, @GuillaumeGomez @QuietMisdreavus -- what's the status? (I did see #58972)

@GuillaumeGomez
Copy link
Member Author

I'm actually not sure myself...

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
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
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
@bors bors closed this in #58972 Apr 11, 2019
@QuietMisdreavus
Copy link
Member

Coming back to this one, i feel like it could be useful, if the check was changed. As-is, this will silence dead links from cross-crate re-exports. (Several of the examples from rand were legitimate dead links from items that were being re-exported.) However, it could be useful to add a check like this to make sure that an item is exported from the current crate before scanning it for links. That way, we don't report an error on something that the user actually doesn't care about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants