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

Do not ICE on AnonConsts in diagnostic_hir_wf_check #124219

Merged
merged 1 commit into from
May 7, 2024

Conversation

gurry
Copy link
Contributor

@gurry gurry commented Apr 21, 2024

Fixes #122989

Below is the snippet from #122989 that ICEs:

trait Traitor<const N: N<2> = 1, const N: N<2> = N> {
    fn N(&N) -> N<2> {
        M
    }
}

trait N<const N: Traitor<2> = 12> {}

The AnonConst that triggers the ICE is the 2 in the param const N: N<2> = 1. The currently existing code in diagnostic_hir_wf_check deals only with AnonConsts that are default values of some param, but the 2 is not a default value. It is just an AnonConst HIR node inside a TraitRef HIR node corresponding to N<2>. Therefore the existing code cannot handle it and this PR ensures that it does.

@rustbot
Copy link
Collaborator

rustbot commented Apr 21, 2024

r? @Nadrieril

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 21, 2024
@Nadrieril
Copy link
Member

r? compiler

@rustbot rustbot assigned nnethercote and unassigned Nadrieril Apr 21, 2024
@compiler-errors
Copy link
Member

@gurry: Please make sure to actually explain in your PR descriptions why something is happening and why your changes are correct, if it's not obvious from the title or the code itself. In this case, it's not really obvious

@compiler-errors
Copy link
Member

r? compiler-errors
@rustbot author

@rustbot rustbot 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 Apr 22, 2024
@gurry
Copy link
Contributor Author

gurry commented Apr 22, 2024

@gurry: Please make sure to actually explain in your PR descriptions why something is happening and why your changes are correct, if it's not obvious from the title or the code itself. In this case, it's not really obvious

Sorry @compiler-errors. Have added the explanation in the description now.

@gurry gurry force-pushed the 122989-ice-unexpected-anon-const branch from 8dd3f4f to 446f78d Compare April 22, 2024 05:05
@gurry
Copy link
Contributor Author

gurry commented Apr 22, 2024

@rustbot label -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 22, 2024
@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented May 7, 2024

📌 Commit 446f78d has been approved by compiler-errors

It is now in the queue for this repository.

@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 7, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 7, 2024
…onst, r=compiler-errors

Do not ICE on `AnonConst`s in `diagnostic_hir_wf_check`

Fixes rust-lang#122989

Below is the snippet from rust-lang#122989 that ICEs:
```rust
trait Traitor<const N: N<2> = 1, const N: N<2> = N> {
    fn N(&N) -> N<2> {
        M
    }
}

trait N<const N: Traitor<2> = 12> {}
```

The `AnonConst` that triggers the ICE is the `2` in the param `const N: N<2> = 1`. The currently existing code in `diagnostic_hir_wf_check` deals only with `AnonConst`s that are default values of some param, but  the `2` is not a default value. It is just an `AnonConst` HIR node inside a `TraitRef` HIR node corresponding to `N<2>`. Therefore the existing code cannot handle it and this PR ensures that it does.
@bors
Copy link
Contributor

bors commented May 7, 2024

⌛ Testing commit 446f78d with merge faefc61...

@bors
Copy link
Contributor

bors commented May 7, 2024

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing faefc61 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 7, 2024
@bors bors merged commit faefc61 into rust-lang:master May 7, 2024
13 checks passed
@rustbot rustbot added this to the 1.80.0 milestone May 7, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (faefc61): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
4.6% [4.6%, 4.6%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.2% [-1.2%, -1.2%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.7% [-1.2%, 4.6%] 2

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 675.093s -> 677.065s (0.29%)
Artifact size: 315.94 MiB -> 315.85 MiB (-0.03%)

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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ICE: Unexpected node AnonConst(AnonConst..
7 participants