Fixed ICE for EII with multiple defaults due to duplicate definition in nameres #150102
+78
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: SATVIKsynopsis <[email protected]>
rustbot
added
S-waiting-on-review
Contributor
|
Thanks for the detailed explanation. that makes a lot of sense. |
Kivooeo
approved these changes
Dec 17, 2025
Contributor
Author
|
@bors r=Kivooeo |
Collaborator
bors
added
S-waiting-on-bors
JonathanBrouwer
added a commit
to JonathanBrouwer/rust
that referenced
this pull request
Dec 18, 2025
Fixed ICE for EII with multiple defaults due to duplicate definition in nameres r? `@jieyouxu` (since you looked at the other one) Fixes rust-lang#149982 Previously a [fix was proposed](rust-lang#149985) by `@SATVIKsynopsis` which I marked as co-author on the first commit for the test they contributed. I'm closing this previous PR. Duplicate definitions of EII defaults shouldn't be possible. I want to still panic on them, since I want to know when other bugs exist. However, in this case the duplicate was caused by something more subtle: both eiis have the same name, and as such a "duplicate definition" error is given. However, the compiler gracefully continues compiling despite that, assuming only one of the two EIIs is actually defined. Both defaults then name resolve, and find the same single remaining EII, and both register themselves to be its default, breaking the single-default assumption. The solution: I added a span-delayed-bug, to make sure we only panic if we hadn't previously had this duplicate definition name resolution error. Thanks to `@SATVIKsynopsis` for their attempt. Adding a diagnostic here could make some sense, but nonetheless I think this is the better solution here <3 Also thanks to `@yaahc` for debugging help, she made me understand the name resolution of the situation so much better and is just lovely in general :3 The last commit is something I tried during debugging, which felt like a relevant test to add (one where both eiis also have the same function name)
Zalathar
added a commit
to Zalathar/rust
that referenced
this pull request
Dec 18, 2025
Fixed ICE for EII with multiple defaults due to duplicate definition in nameres r? ``@jieyouxu`` (since you looked at the other one) Fixes rust-lang#149982 Previously a [fix was proposed](rust-lang#149985) by ``@SATVIKsynopsis`` which I marked as co-author on the first commit for the test they contributed. I'm closing this previous PR. Duplicate definitions of EII defaults shouldn't be possible. I want to still panic on them, since I want to know when other bugs exist. However, in this case the duplicate was caused by something more subtle: both eiis have the same name, and as such a "duplicate definition" error is given. However, the compiler gracefully continues compiling despite that, assuming only one of the two EIIs is actually defined. Both defaults then name resolve, and find the same single remaining EII, and both register themselves to be its default, breaking the single-default assumption. The solution: I added a span-delayed-bug, to make sure we only panic if we hadn't previously had this duplicate definition name resolution error. Thanks to ``@SATVIKsynopsis`` for their attempt. Adding a diagnostic here could make some sense, but nonetheless I think this is the better solution here <3 Also thanks to ``@yaahc`` for debugging help, she made me understand the name resolution of the situation so much better and is just lovely in general :3 The last commit is something I tried during debugging, which felt like a relevant test to add (one where both eiis also have the same function name)
bors
added a commit
that referenced
this pull request
Dec 18, 2025
…uwer Rollup of 5 pull requests Successful merges: - #145933 (Expand `str_as_str` to more types) - #148849 (Set -Cpanic=abort in windows-msvc stack protector tests) - #150048 (std_detect: AArch64 Darwin: expose SME F16F16 and B16B16 features) - #150083 (tests/run-make-cargo/same-crate-name-and-macro-name: New regression test) - #150102 (Fixed ICE for EII with multiple defaults due to duplicate definition in nameres) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
that referenced
this pull request
Dec 18, 2025
…uwer Rollup of 5 pull requests Successful merges: - #145933 (Expand `str_as_str` to more types) - #148849 (Set -Cpanic=abort in windows-msvc stack protector tests) - #150048 (std_detect: AArch64 Darwin: expose SME F16F16 and B16B16 features) - #150083 (tests/run-make-cargo/same-crate-name-and-macro-name: New regression test) - #150102 (Fixed ICE for EII with multiple defaults due to duplicate definition in nameres) r? `@ghost` `@rustbot` modify labels: rollup
bors
added a commit
that referenced
this pull request
Dec 18, 2025
Fixed ICE for EII with multiple defaults due to duplicate definition in nameres r? `@jieyouxu` (since you looked at the other one) Fixes #149982 Previously a [fix was proposed](#149985) by `@SATVIKsynopsis` which I marked as co-author on the first commit for the test they contributed. I'm closing this previous PR. Duplicate definitions of EII defaults shouldn't be possible. I want to still panic on them, since I want to know when other bugs exist. However, in this case the duplicate was caused by something more subtle: both eiis have the same name, and as such a "duplicate definition" error is given. However, the compiler gracefully continues compiling despite that, assuming only one of the two EIIs is actually defined. Both defaults then name resolve, and find the same single remaining EII, and both register themselves to be its default, breaking the single-default assumption. The solution: I added a span-delayed-bug, to make sure we only panic if we hadn't previously had this duplicate definition name resolution error. Thanks to `@SATVIKsynopsis` for their attempt. Adding a diagnostic here could make some sense, but nonetheless I think this is the better solution here <3 Also thanks to `@yaahc` for debugging help, she made me understand the name resolution of the situation so much better and is just lovely in general :3 The last commit is something I tried during debugging, which felt like a relevant test to add (one where both eiis also have the same function name)
Collaborator
Collaborator
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
r? @jieyouxu (since you looked at the other one)
Fixes #149982
Previously a fix was proposed by @SATVIKsynopsis which I marked as co-author on the first commit for the test they contributed. I'm closing this previous PR.
Duplicate definitions of EII defaults shouldn't be possible. I want to still panic on them, since I want to know when other bugs exist. However, in this case the duplicate was caused by something more subtle: both eiis have the same name, and as such a "duplicate definition" error is given. However, the compiler gracefully continues compiling despite that, assuming only one of the two EIIs is actually defined.
Both defaults then name resolve, and find the same single remaining EII, and both register themselves to be its default, breaking the single-default assumption.
The solution: I added a span-delayed-bug, to make sure we only panic if we hadn't previously had this duplicate definition name resolution error.
Thanks to @SATVIKsynopsis for their attempt. Adding a diagnostic here could make some sense, but nonetheless I think this is the better solution here <3
Also thanks to @yaahc for debugging help, she made me understand the name resolution of the situation so much better and is just lovely in general :3
The last commit is something I tried during debugging, which felt like a relevant test to add (one where both eiis also have the same function name)