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

RustDoc: Fix bounds linking trait.Foo instead of traitalias.Foo #84811

Merged
merged 2 commits into from
May 3, 2021

Conversation

scottmcm
Copy link
Member

@scottmcm scottmcm commented May 2, 2021

Fixes #84782

The code was assuming Trait when adding bounds to the cache, so add a check on the DefId to see what its kind really is.

r? @jyn514

Before:
image

After:
image

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 2, 2021
@jyn514
Copy link
Member

jyn514 commented May 2, 2021

My review todo list:

  • audit all the wildcards in rustdoc
  • open an issue for getting rid of separate HIR handling
  • audit all uses of ItemKind::Trait to see if they should handle TraitAlias as well

@jyn514 jyn514 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label May 2, 2021
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't had time to do all the things on #84811 (comment), but I think we can land this in the meantime. Just a couple nits.

src/librustdoc/clean/mod.rs Outdated Show resolved Hide resolved
src/test/rustdoc/trait-alias-mention.rs Outdated Show resolved Hide resolved
}
}
clean::PrimitiveItem(..) => {
self.cache.paths.insert(item.def_id, (self.cache.stack.clone(), item.type_()));
}

_ => {}
clean::ExternCrateItem { .. }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to work ok as a re-export:
image
I don't think extern crates can show up anywhere else, so this seems fine for now.

_ => {}
clean::ExternCrateItem { .. }
| clean::ImportItem(..)
| clean::OpaqueTyItem(..)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to work ok:
image
image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, is this one TAIT? I would have guessed https://doc.rust-lang.org/nightly/unstable-book/language-features/extern-types.html

But overall, I'd like to leave the match as-is for this PR, if that's ok. Then someone else can audit them later and move them to different sections with comments about why they're ok to be skipped, or whatever.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that seems good to me.

@jyn514 jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 2, 2021
@rust-log-analyzer

This comment has been minimized.

@jyn514
Copy link
Member

jyn514 commented May 2, 2021

@bors r+

@bors
Copy link
Contributor

bors commented May 2, 2021

📌 Commit 40ffa94 has been approved by jyn514

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 2, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request May 3, 2021
Rollup of 6 pull requests

Successful merges:

 - rust-lang#84072 (Allow setting `target_family` to multiple values, and implement `target_family="wasm"`)
 - rust-lang#84744 (Add ErrorKind::OutOfMemory)
 - rust-lang#84784 (Add help message to suggest const for unused type param)
 - rust-lang#84811 (RustDoc: Fix bounds linking trait.Foo instead of traitalias.Foo)
 - rust-lang#84818 (suggestion for unit enum variant when matched with a patern)
 - rust-lang#84832 (Do not print visibility in external traits)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 37ce332 into rust-lang:master May 3, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 3, 2021
@scottmcm scottmcm deleted the rustdoc-trait-alias-fix branch May 3, 2021 03:55
bors added a commit to rust-lang-ci/rust that referenced this pull request May 18, 2021
Implement the new desugaring from `try_trait_v2`

~~Currently blocked on rust-lang#84782, which has a PR in rust-lang#84811 Rebased atop that fix.

`try_trait_v2` tracking issue: rust-lang#84277

Unfortunately this is already touching a ton of things, so if you have suggestions for good ways to split it up, I'd be happy to hear them.  (The combination between the use in the library, the compiler changes, the corresponding diagnostic differences, even MIR tests mean that I don't really have a great plan for it other than trying to have decently-readable commits.

r? `@ghost`

~~(This probably shouldn't go in during the last week before the fork anyway.)~~ Fork happened.
flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request May 20, 2021
Implement the new desugaring from `try_trait_v2`

~~Currently blocked on rust-lang/rust#84782, which has a PR in rust-lang/rust#84811 Rebased atop that fix.

`try_trait_v2` tracking issue: rust-lang/rust#84277

Unfortunately this is already touching a ton of things, so if you have suggestions for good ways to split it up, I'd be happy to hear them.  (The combination between the use in the library, the compiler changes, the corresponding diagnostic differences, even MIR tests mean that I don't really have a great plan for it other than trying to have decently-readable commits.

r? `@ghost`

~~(This probably shouldn't go in during the last week before the fork anyway.)~~ Fork happened.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RustDoc: Links to trait aliases go to trait.Foo.html but the pages are traitalias.Foo.html
7 participants