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 generic impl rustdoc json output #98053

Merged
merged 2 commits into from
Jun 16, 2022

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Jun 13, 2022

Fixes #97986.

The problem in case of generic trait impl is that the trait's items are the same for all the types afterward. But since they're the same, it's safe for rustdoc-json to just ignore them.

A little representation of what's going on:

trait T {
    fn f(); // <- defid 0
}

impl<Y> T for Y {
    fn f() {} // <- defid 1
}

struct S; // <- defid 1 (since it matches `impl<Y> T for Y`

cc @Urgau

r? @CraftSpider

@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 13, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 13, 2022
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member Author

And fixed fmt...

@Enselic
Copy link
Member

Enselic commented Jun 13, 2022

Thanks a lot for this fix.

I can confirm that this makes it possible to build rustdoc JSON for https://github.com/serde-rs/serde/tree/master/serde. It used to result in an ICE.

There is still an ICE for https://github.com/diesel-rs/diesel/tree/b5c2e26d4d9f464458430427704fc50de985c632/diesel though. Just wanted to mention it in case you want to take care of that while you're at it. It is also triggered by a blanket impl, see analysis in #93588 (comment) and #93588 (comment).

@GuillaumeGomez
Copy link
Member Author

I have the three other issues open. I will go through them after this one is merged.

@GuillaumeGomez
Copy link
Member Author

Oh right, I forgot @CraftSpider was away for the moment. I'll set another reviewer.

r? @notriddle

@notriddle
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 15, 2022

📌 Commit 99cd9ca 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 15, 2022
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 15, 2022
…n-ice, r=notriddle

Fix generic impl rustdoc json output

Fixes rust-lang#97986.

The problem in case of generic trait impl is that the trait's items are the same for all the types afterward. But since they're the same, it's safe for rustdoc-json to just ignore them.

A little representation of what's going on:

```rust
trait T {
    fn f(); // <- defid 0
}

impl<Y> T for Y {
    fn f() {} // <- defid 1
}

struct S; // <- defid 1 (since it matches `impl<Y> T for Y`
```

cc `@Urgau`

r? `@CraftSpider`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 15, 2022
…n-ice, r=notriddle

Fix generic impl rustdoc json output

Fixes rust-lang#97986.

The problem in case of generic trait impl is that the trait's items are the same for all the types afterward. But since they're the same, it's safe for rustdoc-json to just ignore them.

A little representation of what's going on:

```rust
trait T {
    fn f(); // <- defid 0
}

impl<Y> T for Y {
    fn f() {} // <- defid 1
}

struct S; // <- defid 1 (since it matches `impl<Y> T for Y`
```

cc ``@Urgau``

r? ``@CraftSpider``
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 15, 2022
Rollup of 7 pull requests

Successful merges:

 - rust-lang#97202 (os str capacity documentation)
 - rust-lang#97964 (Fix suggestions for `&a: T` parameters)
 - rust-lang#98053 (Fix generic impl rustdoc json output)
 - rust-lang#98059 (Inline `const_eval_select`)
 - rust-lang#98092 (Fix sidebar items expand collapse)
 - rust-lang#98119 (Refactor path segment parameter error)
 - rust-lang#98135 (Add regression test for rust-lang#93775)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4ee78a6 into rust-lang:master Jun 16, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 16, 2022
@GuillaumeGomez GuillaumeGomez deleted the fix-generic-impl-json-ice branch June 16, 2022 09:08
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 28, 2022
…doc-json, r=GuillaumeGomez

rustdoc-json: Allow Typedef to be different in sanity assert

Closes rust-lang#98547

This fix is a natural extension of rust-lang#98053.

r? `@notriddle`

(Since you reviewed the other PR.)

CC `@GuillaumeGomez`

`@rustbot` labels +A-rustdoc-json +T-rustdoc
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: ICE when same item is represented in different ways
8 participants