Skip to content

Commit 380d3a1

Browse files
committed
Auto 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)
2 parents ed0006a + 08b7d34 commit 380d3a1

File tree

5 files changed

+78
-1
lines changed

5 files changed

+78
-1
lines changed

compiler/rustc_passes/src/eii.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,8 @@ pub(crate) fn check_externally_implementable_items<'tcx>(tcx: TyCtxt<'tcx>, ():
116116
}
117117

118118
if default_impls.len() > 1 {
119-
panic!("multiple not supported right now");
119+
let decl_span = tcx.def_ident_span(decl_did).unwrap();
120+
tcx.dcx().span_delayed_bug(decl_span, "multiple not supported right now");
120121
}
121122

122123
let (local_impl, is_default) =
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
#![crate_type = "lib"]
2+
#![feature(extern_item_impls)]
3+
// `eii` expands to, among other things, `macro eii() {}`.
4+
// If we have two eiis named the same thing, we have a duplicate definition
5+
// for that macro. The compiler happily continues compiling on duplicate
6+
// definitions though, to emit as many diagnostics as possible.
7+
// However, in the case of eiis, this can break the assumption that every
8+
// eii has only one default implementation, since the default for both eiis will
9+
// name resolve to the same eii definiton (since the other definition was duplicate)
10+
// This test tests for the previously-ICE that occurred when this assumption
11+
// (of 1 default) was broken which was reported in #149982.
12+
13+
#[eii(eii1)]
14+
fn a() {}
15+
16+
#[eii(eii1)]
17+
//~^ ERROR the name `eii1` is defined multiple times
18+
fn a() {}
19+
//~^ ERROR the name `a` is defined multiple times
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
error[E0428]: the name `a` is defined multiple times
2+
--> $DIR/multiple-default-impls-same-name.rs:18:1
3+
|
4+
LL | fn a() {}
5+
| ------ previous definition of the value `a` here
6+
...
7+
LL | fn a() {}
8+
| ^^^^^^ `a` redefined here
9+
|
10+
= note: `a` must be defined only once in the value namespace of this module
11+
12+
error[E0428]: the name `eii1` is defined multiple times
13+
--> $DIR/multiple-default-impls-same-name.rs:16:1
14+
|
15+
LL | #[eii(eii1)]
16+
| ------------ previous definition of the macro `eii1` here
17+
...
18+
LL | #[eii(eii1)]
19+
| ^^^^^^^^^^^^ `eii1` redefined here
20+
|
21+
= note: `eii1` must be defined only once in the macro namespace of this module
22+
23+
error: aborting due to 2 previous errors
24+
25+
For more information about this error, try `rustc --explain E0428`.
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
#![crate_type = "lib"]
2+
#![feature(extern_item_impls)]
3+
// `eii` expands to, among other things, `macro eii() {}`.
4+
// If we have two eiis named the same thing, we have a duplicate definition
5+
// for that macro. The compiler happily continues compiling on duplicate
6+
// definitions though, to emit as many diagnostics as possible.
7+
// However, in the case of eiis, this can break the assumption that every
8+
// eii has only one default implementation, since the default for both eiis will
9+
// name resolve to the same eii definiton (since the other definition was duplicate)
10+
// This test tests for the previously-ICE that occurred when this assumption
11+
// (of 1 default) was broken which was reported in #149982.
12+
13+
#[eii(eii1)]
14+
fn a() {}
15+
16+
#[eii(eii1)]
17+
//~^ ERROR the name `eii1` is defined multiple times
18+
fn b() {}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error[E0428]: the name `eii1` is defined multiple times
2+
--> $DIR/multiple-default-impls.rs:16:1
3+
|
4+
LL | #[eii(eii1)]
5+
| ------------ previous definition of the macro `eii1` here
6+
...
7+
LL | #[eii(eii1)]
8+
| ^^^^^^^^^^^^ `eii1` redefined here
9+
|
10+
= note: `eii1` must be defined only once in the macro namespace of this module
11+
12+
error: aborting due to 1 previous error
13+
14+
For more information about this error, try `rustc --explain E0428`.

0 commit comments

Comments
 (0)