fixed ICE for more than one eii implementations #149985

SATVIKsynopsis · 2025-12-14T11:55:51Z

this pr fixes an ice issue which was there by having multiple default eii implementations. the compiler previously panicked in this situation but now it gives a clear error instead.

also, a ui test is added to cover this case and prevent any implications.

rustbot · 2025-12-14T11:55:55Z

r? @jieyouxu

rustbot has assigned @jieyouxu.
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

…r standards

rust-log-analyzer · 2025-12-14T17:04:02Z

The job pr-check-1 failed! Check out the build log: (web) (plain enhanced) (plain)

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

<Nothing changed>
+ reuse --include-submodules lint
# MISSING COPYRIGHT AND LICENSING INFORMATION

The following files have no copyright and licensing information:
* ../e -i HEAD~2


# SUMMARY

* Bad licenses: 0
---


# RECOMMENDATIONS

* Fix missing copyright/licensing information: For one or more files, the tool
  cannot find copyright and/or licensing information. You typically do this by
  adding 'SPDX-FileCopyrightText' and 'SPDX-License-Identifier' tags to each
  file. The tutorial explains additional ways to do this:
  <https://reuse.software/tutorial/>

  local time: Sun Dec 14 17:00:25 UTC 2025
  network time: Sun, 14 Dec 2025 17:00:25 GMT
##[error]Process completed with exit code 1.
##[group]Run echo "disk usage:"

jieyouxu · 2025-12-15T00:58:47Z

I'm not sure if this is just some temporarily placeholder with the intention that it will be eventually supported. If so, in that case, emitting some specific diagnostics doesn't feel necessary, the panic is fine. Not sure about the context, so maybe

r? @jdonszelmann (if available)

jdonszelmann · 2025-12-15T01:05:04Z

We likely cannot ever support this, so sure, it's probably alright to make this into a diagnostic not a panic.

jdonszelmann · 2025-12-15T01:05:26Z

Could you please turn it into a structured diagnostic though like the other ones around here?

jdonszelmann · 2025-12-15T01:10:25Z

Nooo, wait a second, stop, this isn't what's going on here. There's a deeper bug here. This diagnostic is I guess fine, though there will never be a stable way to achieve this since adding a diagnostic out separated from the declaration won't be something you'd normally do.

The problem is that this won't fix #149982. That code shouldn't get here. Please give me a little bit of time researching that and then we can maybe move forward here. I don't want to accidentally hide an otherwise real bug.

jdonszelmann · 2025-12-15T01:10:44Z

@rustbot blocked

jdonszelmann · 2025-12-15T01:11:51Z

@jieyouxu I'm available for any review surrounding eiis, and probably atm the only one who should given how experimental this one is. If you see more like it feel free to assign me <3

jdonszelmann · 2025-12-15T01:14:37Z

Oh and this one will have to be squashed in any case fyi

SATVIKsynopsis · 2025-12-15T03:30:39Z

Thanks for the clarification. That makes sense. I agree it would be better not to mask a deeper issue. Happy to wait and help investigate once you have had time to look into it.

jdonszelmann · 2025-12-17T17:12:51Z

Closing in favor of #150102

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)

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)

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)

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)

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)

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)

fixed ICE for more than one eii implementations

16f6cf8

rustbot assigned jieyouxu Dec 14, 2025

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 14, 2025

This comment has been minimized.

Sign in to view

SATVIKsynopsis added 2 commits December 14, 2025 17:36

tidy fix done

ddda03c

chneges return to continue to better diagnostic checkings and compile…

534c9e2

…r standards

This comment has been minimized.

Sign in to view

CI + diagnostics fix

830d294

rustbot assigned jdonszelmann and unassigned jieyouxu Dec 15, 2025

rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 15, 2025

jdonszelmann mentioned this pull request Dec 17, 2025

Fixed ICE for EII with multiple defaults due to duplicate definition in nameres #150102

Merged

jdonszelmann closed this Dec 17, 2025

Uh oh!

fixed ICE for more than one eii implementations #149985

fixed ICE for more than one eii implementations #149985

Conversation

SATVIKsynopsis commented Dec 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Dec 14, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-log-analyzer commented Dec 14, 2025

Uh oh!

jieyouxu commented Dec 15, 2025

Uh oh!

jdonszelmann commented Dec 15, 2025

Uh oh!

jdonszelmann commented Dec 15, 2025

Uh oh!

jdonszelmann commented Dec 15, 2025

Uh oh!

jdonszelmann commented Dec 15, 2025

Uh oh!

jdonszelmann commented Dec 15, 2025

Uh oh!

jdonszelmann commented Dec 15, 2025

Uh oh!

SATVIKsynopsis commented Dec 15, 2025

Uh oh!

jdonszelmann commented Dec 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SATVIKsynopsis commented Dec 14, 2025 •

edited

Loading