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

Fix "kind" for associated types in trait implementations in rustdoc JSON #98577

Merged
merged 2 commits into from
Jun 28, 2022

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Jun 27, 2022

Fixes #81340.

Contrary to what is suggested in the issue, I really think we should distinguish between associated items and "normal" constants and types.

cc @CraftSpider @SimonSapin
r? @notriddle

@GuillaumeGomez GuillaumeGomez added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-json Area: Rustdoc JSON backend labels Jun 27, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 27, 2022
@notriddle
Copy link
Contributor

Am I correct in guessing that this PR is an alternative to #98548 ?

@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Jun 27, 2022

Yes.

EDIT: Ok extra explanations: since we're now generating the correct AssocType instead of typedef, so it will enter https://github.com/rust-lang/rust/blob/master/src/librustdoc/json/mod.rs#L203-L204, preventing the panic. I'll add a test for it too.

@notriddle
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 27, 2022

📌 Commit 9277f95 has been approved by notriddle

@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 Jun 27, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 27, 2022
…askrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#98331 (Fix rustdoc argument error)
 - rust-lang#98506 (Fix span issues in object safety suggestions)
 - rust-lang#98563 (interpret: refactor allocation info query)
 - rust-lang#98576 (small regions refactoring)
 - rust-lang#98577 (Fix "kind" for associated types in trait implementations in rustdoc JSON)
 - rust-lang#98578 (Remove eddyb from miri failure pings)
 - rust-lang#98579 (liballoc tests: avoid int2ptr cast)
 - rust-lang#98581 (Add triagebot mentions.)
 - rust-lang#98587 (libcore tests: avoid int2ptr casts)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d0ae6eb into rust-lang:master Jun 28, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jun 28, 2022
@GuillaumeGomez GuillaumeGomez deleted the associated-items branch June 28, 2022 08:36
@@ -252,8 +252,11 @@ fn from_clean_item(item: clean::Item, tcx: TyCtxt<'_>) -> ItemEnum {
bounds: b.into_iter().map(|x| x.into_tcx(tcx)).collect(),
default: None,
},
// FIXME: do not map to Typedef but to a custom variant
AssocTypeItem(t, _) => ItemEnum::Typedef(t.into_tcx(tcx)),
AssocTypeItem(t, b) => ItemEnum::AssocType {
Copy link
Member

Choose a reason for hiding this comment

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

My nightly regression test job for cargo public-api found a regression with this change (job)

Reported as #98658

AssocTypeItem(t, b) => ItemEnum::AssocType {
generics: t.generics.into_tcx(tcx),
bounds: b.into_iter().map(|x| x.into_tcx(tcx)).collect(),
default: t.item_type.map(|ty| ty.into_tcx(tcx)),
Copy link
Member

Choose a reason for hiding this comment

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

Comparing with the HTML rendering code I think this line should be something like

default: t.item_type.unwrap_or(t.type_).map(|ty| ty.into_tcx(tcx))

I will maybe have time later today to try that fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-json Area: Rustdoc JSON backend 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 json: Associated constants in impl have wrong representation
6 participants