Skip to content

Conversation

@jdonszelmann
Copy link
Contributor

@jdonszelmann jdonszelmann commented Dec 17, 2025

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)

@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 Dec 17, 2025
@SATVIKsynopsis
Copy link
Contributor

SATVIKsynopsis commented Dec 17, 2025

Thanks for the detailed explanation. that makes a lot of sense.
Glad the root cause is clearer now, and happy the tests were useful.

Copy link
Member

@Kivooeo Kivooeo left a comment

Choose a reason for hiding this comment

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

This change replaces panic with span_delayed_bug, which seems like a good improvement based on the research Jana shared. I don’t foresee any issues from my side

feel free r=me on this

View changes since this review

@jdonszelmann
Copy link
Contributor Author

@bors r=Kivooeo

@bors
Copy link
Collaborator

bors commented Dec 17, 2025

📌 Commit 08b7d34 has been approved by Kivooeo

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 Dec 17, 2025
@jieyouxu jieyouxu assigned Kivooeo and unassigned jieyouxu Dec 17, 2025
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)
@bors
Copy link
Collaborator

bors commented Dec 18, 2025

⌛ Testing commit 08b7d34 with merge 380d3a1...

@bors
Copy link
Collaborator

bors commented Dec 18, 2025

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 18, 2025
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
@JonathanBrouwer
Copy link
Contributor

@bors retry (Spurious)

@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 Dec 18, 2025
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

#1 [internal] booting buildkit
#1 pulling image ghcr.io/rust-lang/buildkit:buildx-stable-1
#1 pulling image ghcr.io/rust-lang/buildkit:buildx-stable-1 15.1s done
#1 ERROR: Error response from daemon: Head "https://ghcr.io/v2/rust-lang/buildkit/manifests/buildx-stable-1": Get "https://ghcr.io/token?account=rust-lang&scope=repository%3Arust-lang%2Fbuildkit%3Apull&service=ghcr.io": net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)
------
 > [internal] booting buildkit:
------
ERROR: failed to build: Error response from daemon: Head "https://ghcr.io/v2/rust-lang/buildkit/manifests/buildx-stable-1": Get "https://ghcr.io/token?account=rust-lang&scope=repository%3Arust-lang%2Fbuildkit%3Apull&service=ghcr.io": net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)
Command failed. Attempt 2/5:
#0 building with "elastic_williams" instance using docker-container driver

#1 [internal] booting buildkit
#1 pulling image ghcr.io/rust-lang/buildkit:buildx-stable-1
---
36c2692d9efd: Retrying in 3 seconds
36c2692d9efd: Retrying in 2 seconds
36c2692d9efd: Retrying in 1 second
36c2692d9efd: Pushed
Head "https://ghcr.io/v2/rust-lang/rust-ci/blobs/sha256:a38eab7450002b16e68075d089f670b86756adc594338543eb2634413e5bbf14": dial tcp 140.82.116.33:443: i/o timeout
##[error]Process completed with exit code 1.
##[group]Run echo "disk usage:"
echo "disk usage:"
df -h
shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}

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
@jdonszelmann
Copy link
Contributor Author

spurious again hehe

@jdonszelmann
Copy link
Contributor Author

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Dec 18, 2025

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Collaborator

bors commented Dec 18, 2025

📌 Commit 08b7d34 has been approved by jdonszelmann

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Dec 18, 2025

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

@jdonszelmann
Copy link
Contributor Author

@bors r-
@bors r=Kivooeo

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 18, 2025
@bors
Copy link
Collaborator

bors commented Dec 18, 2025

📌 Commit 08b7d34 has been approved by Kivooeo

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Dec 18, 2025

🌲 The tree is currently closed for pull requests below priority 1000. This pull request will be tested once the tree is reopened.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 18, 2025
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
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)
bors added a commit that referenced this pull request Dec 18, 2025
…uwer

Rollup of 11 pull requests

Successful merges:

 - #145933 (Expand `str_as_str` to more types)
 - #148849 (Set -Cpanic=abort in windows-msvc stack protector tests)
 - #149925 (`cfg_select!`: parse unused branches)
 - #150022 (Generate macro expansion for rust compiler crates docs)
 - #150024 (Support recursive delegation)
 - #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)
 - #150124 (unstable.rs: fix typos in comments (implementatble -> implementable))
 - #150125 (Port `#[rustc_lint_opt_deny_field_access]` to attribute parser)
 - #150126 (Subtree sync for rustc_codegen_cranelift)

Failed merges:

 - #150127 (Port `#[rustc_lint_untracked_query_information]` and `#[rustc_lint_diagnostics]` to using attribute parsers)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Dec 18, 2025
…uwer

Rollup of 12 pull requests

Successful merges:

 - #145933 (Expand `str_as_str` to more types)
 - #148849 (Set -Cpanic=abort in windows-msvc stack protector tests)
 - #149925 (`cfg_select!`: parse unused branches)
 - #149952 (Suggest struct pattern when destructuring Range with .. syntax)
 - #150022 (Generate macro expansion for rust compiler crates docs)
 - #150024 (Support recursive delegation)
 - #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)
 - #150124 (unstable.rs: fix typos in comments (implementatble -> implementable))
 - #150125 (Port `#[rustc_lint_opt_deny_field_access]` to attribute parser)
 - #150126 (Subtree sync for rustc_codegen_cranelift)

Failed merges:

 - #150127 (Port `#[rustc_lint_untracked_query_information]` and `#[rustc_lint_diagnostics]` to using attribute parsers)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4c9451b into rust-lang:main Dec 18, 2025
11 of 12 checks passed
@rustbot rustbot added this to the 1.94.0 milestone Dec 18, 2025
rust-timer added a commit that referenced this pull request Dec 18, 2025
Rollup merge of #150102 - jdonszelmann:fix-149982, r=Kivooeo

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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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 eii: multiple annotations

8 participants