Skip to content

rustdoc: render <Self as X>::Y type casts properly #85479

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

Merged
merged 2 commits into from
May 23, 2021

Conversation

Stupremee
Copy link
Member

Rustdoc didn't render any <Self as X> casts which causes invalid code inside the documentation. This is fixed by this PR by checking if the target type X is different from Self, and if so, it will render a typecast.

Resolves #85454

@rust-highfive
Copy link
Contributor

r? @CraftSpider

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 19, 2021
Type::QPath {
name: cx.tcx.associated_item(self.item_def_id).ident.name,
self_type: box self.self_ty().clean(cx),
self_def_id: self_type.def_id(),
self_type: box self_type,
Copy link
Member

Choose a reason for hiding this comment

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

Why add a separate def_id field? You can just call self_type.def_id() wherever you need it.

Copy link
Member Author

@Stupremee Stupremee May 20, 2021

Choose a reason for hiding this comment

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

because self_type is a Generic(Symbol("Self")) most of the time, and it was much easier to pass is that way. I also have a git stash that adds a new Type called SelfType, but it was much more work to implement and still has some bugs.

Copy link
Contributor

Choose a reason for hiding this comment

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

I personally feel a bit uncomfortable with the added field, would it make sense to add a helper function/method somewhere instead?

Copy link
Member Author

@Stupremee Stupremee May 23, 2021

Choose a reason for hiding this comment

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

The only reason the field exists that the DefId of the Self type is lost after cleaning because the Self generic parameter is resolved to Generic("Self"), which does not contain any DefId, but a DefId is required to check if the cast should be displayed.
I don't know how a new helper function would help with that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, sorry, I misread your above comment. Request withdrawn.

@CraftSpider
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented May 23, 2021

📌 Commit d637ed4 has been approved by CraftSpider

@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-review Status: Awaiting review from the assignee but also interested parties. labels May 23, 2021
@bors
Copy link
Collaborator

bors commented May 23, 2021

⌛ Testing commit d637ed4 with merge 13bf0b2...

@bors
Copy link
Collaborator

bors commented May 23, 2021

☀️ Test successful - checks-actions
Approved by: CraftSpider
Pushing 13bf0b2 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 23, 2021
@bors bors merged commit 13bf0b2 into rust-lang:master May 23, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustdoc removes Try from <Self as Try>::Residual in std::ops::FromResidual
6 participants