From bd8e88fd7b2401242cbcc95d88ef7b8058d59a77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 19 Jul 2024 19:48:21 +0000 Subject: [PATCH 1/2] Do not ICE with incorrect empty suggestion When we have two types with the same name, one without type parameters and the other with type parameters and a derive macro, we were before incorrectly suggesting to remove type parameters from the former, which ICEd because we were suggesting to remove nothing. We now gate against this. The output is still not perfect. E0107 should explicitly detect this case and provide better context, but for now let's avoid the ICE. --- .../errors/wrong_number_of_generic_args.rs | 12 +++- ...ultiple-types-with-same-name-and-derive.rs | 17 +++++ ...ple-types-with-same-name-and-derive.stderr | 71 +++++++++++++++++++ 3 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs create mode 100644 tests/ui/duplicate/multiple-types-with-same-name-and-derive.stderr diff --git a/compiler/rustc_hir_analysis/src/errors/wrong_number_of_generic_args.rs b/compiler/rustc_hir_analysis/src/errors/wrong_number_of_generic_args.rs index 97402dd1109f3..418f12b715801 100644 --- a/compiler/rustc_hir_analysis/src/errors/wrong_number_of_generic_args.rs +++ b/compiler/rustc_hir_analysis/src/errors/wrong_number_of_generic_args.rs @@ -1048,7 +1048,17 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> { }, ); - err.span_suggestion(span, msg, "", Applicability::MaybeIncorrect); + if span.is_empty() { + // Avoid ICE when types with the same name with `derive`s are in the same scope: + // struct NotSM; + // #[derive(PartialEq, Eq)] + // struct NotSM(T); + // With the above code, the suggestion is to remove the generics of the first + // `NotSM`, which doesn't *have* generics, so we're suggesting to remove no code + // with no code, which ICEs on nightly due to an `assert!`. + } else { + err.span_suggestion(span, msg, "", Applicability::MaybeIncorrect); + } } else if redundant_lifetime_args && redundant_type_or_const_args { remove_lifetime_args(err); remove_type_or_const_args(err); diff --git a/tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs b/tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs new file mode 100644 index 0000000000000..93c25b243e225 --- /dev/null +++ b/tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs @@ -0,0 +1,17 @@ +// Here, there are two types with the same name. One of these has a `derive` annotation, but in the +// expansion these `impl`s are associated to the the *other* type. There is a suggestion to remove +// unneded type parameters, but because we're now point at a type with no type parameters, the +// suggestion would suggest removing code from an empty span, which would ICE in nightly. + +struct NotSM; + +#[derive(PartialEq, Eq)] +//~^ ERROR struct takes 0 generic arguments +//~| ERROR struct takes 0 generic arguments +//~| ERROR struct takes 0 generic arguments +//~| ERROR struct takes 0 generic arguments +struct NotSM(T); +//~^ ERROR the name `NotSM` is defined multiple times +//~| ERROR no field `0` + +fn main() {} diff --git a/tests/ui/duplicate/multiple-types-with-same-name-and-derive.stderr b/tests/ui/duplicate/multiple-types-with-same-name-and-derive.stderr new file mode 100644 index 0000000000000..e3aeb36206360 --- /dev/null +++ b/tests/ui/duplicate/multiple-types-with-same-name-and-derive.stderr @@ -0,0 +1,71 @@ +error[E0428]: the name `NotSM` is defined multiple times + --> $DIR/multiple-types-with-same-name-and-derive.rs:13:1 + | +LL | struct NotSM; + | ------------- previous definition of the type `NotSM` here +... +LL | struct NotSM(T); + | ^^^^^^^^^^^^^^^^^^^ `NotSM` redefined here + | + = note: `NotSM` must be defined only once in the type namespace of this module + +error[E0107]: struct takes 0 generic arguments but 1 generic argument was supplied + --> $DIR/multiple-types-with-same-name-and-derive.rs:8:10 + | +LL | #[derive(PartialEq, Eq)] + | ^^^^^^^^^ expected 0 generic arguments + | +note: struct defined here, with 0 generic parameters + --> $DIR/multiple-types-with-same-name-and-derive.rs:6:8 + | +LL | struct NotSM; + | ^^^^^ + +error[E0107]: struct takes 0 generic arguments but 1 generic argument was supplied + --> $DIR/multiple-types-with-same-name-and-derive.rs:8:10 + | +LL | #[derive(PartialEq, Eq)] + | ^^^^^^^^^ expected 0 generic arguments + | +note: struct defined here, with 0 generic parameters + --> $DIR/multiple-types-with-same-name-and-derive.rs:6:8 + | +LL | struct NotSM; + | ^^^^^ + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error[E0107]: struct takes 0 generic arguments but 1 generic argument was supplied + --> $DIR/multiple-types-with-same-name-and-derive.rs:8:21 + | +LL | #[derive(PartialEq, Eq)] + | ^^ expected 0 generic arguments + | +note: struct defined here, with 0 generic parameters + --> $DIR/multiple-types-with-same-name-and-derive.rs:6:8 + | +LL | struct NotSM; + | ^^^^^ + +error[E0107]: struct takes 0 generic arguments but 1 generic argument was supplied + --> $DIR/multiple-types-with-same-name-and-derive.rs:8:10 + | +LL | #[derive(PartialEq, Eq)] + | ^^^^^^^^^ expected 0 generic arguments + | +note: struct defined here, with 0 generic parameters + --> $DIR/multiple-types-with-same-name-and-derive.rs:6:8 + | +LL | struct NotSM; + | ^^^^^ + = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` + +error[E0609]: no field `0` on type `&NotSM` + --> $DIR/multiple-types-with-same-name-and-derive.rs:13:17 + | +LL | struct NotSM(T); + | ^ unknown field + +error: aborting due to 6 previous errors + +Some errors have detailed explanations: E0107, E0428, E0609. +For more information about an error, try `rustc --explain E0107`. From 682c5f485b4cc55ce16315e0d76a5a16ab3064f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Wed, 18 Sep 2024 19:30:20 +0200 Subject: [PATCH 2/2] Explicitly mark a hack as a HACK and elaborate its comment --- .../errors/wrong_number_of_generic_args.rs | 15 +++++++------- ...ultiple-types-with-same-name-and-derive.rs | 2 ++ ...ple-types-with-same-name-and-derive.stderr | 20 +++++++++---------- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/errors/wrong_number_of_generic_args.rs b/compiler/rustc_hir_analysis/src/errors/wrong_number_of_generic_args.rs index 418f12b715801..236543007fc3f 100644 --- a/compiler/rustc_hir_analysis/src/errors/wrong_number_of_generic_args.rs +++ b/compiler/rustc_hir_analysis/src/errors/wrong_number_of_generic_args.rs @@ -1049,13 +1049,14 @@ impl<'a, 'tcx> WrongNumberOfGenericArgs<'a, 'tcx> { ); if span.is_empty() { - // Avoid ICE when types with the same name with `derive`s are in the same scope: - // struct NotSM; - // #[derive(PartialEq, Eq)] - // struct NotSM(T); - // With the above code, the suggestion is to remove the generics of the first - // `NotSM`, which doesn't *have* generics, so we're suggesting to remove no code - // with no code, which ICEs on nightly due to an `assert!`. + // HACK: Avoid ICE when types with the same name with `derive`s are in the same scope: + // struct NotSM; + // #[derive(PartialEq, Eq)] + // struct NotSM(T); + // With the above code, the suggestion would be to remove the generics of the first + // `NotSM`, which doesn't *have* generics, so we would suggest to remove no code with + // no code, which would trigger an `assert!` later. Ideally, we would do something a + // bit more principled. See closed PR #109082. } else { err.span_suggestion(span, msg, "", Applicability::MaybeIncorrect); } diff --git a/tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs b/tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs index 93c25b243e225..8d36981b41b2f 100644 --- a/tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs +++ b/tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs @@ -2,6 +2,8 @@ // expansion these `impl`s are associated to the the *other* type. There is a suggestion to remove // unneded type parameters, but because we're now point at a type with no type parameters, the // suggestion would suggest removing code from an empty span, which would ICE in nightly. +// +// issue: rust-lang/rust#108748 struct NotSM; diff --git a/tests/ui/duplicate/multiple-types-with-same-name-and-derive.stderr b/tests/ui/duplicate/multiple-types-with-same-name-and-derive.stderr index e3aeb36206360..32c3cf4403169 100644 --- a/tests/ui/duplicate/multiple-types-with-same-name-and-derive.stderr +++ b/tests/ui/duplicate/multiple-types-with-same-name-and-derive.stderr @@ -1,5 +1,5 @@ error[E0428]: the name `NotSM` is defined multiple times - --> $DIR/multiple-types-with-same-name-and-derive.rs:13:1 + --> $DIR/multiple-types-with-same-name-and-derive.rs:15:1 | LL | struct NotSM; | ------------- previous definition of the type `NotSM` here @@ -10,57 +10,57 @@ LL | struct NotSM(T); = note: `NotSM` must be defined only once in the type namespace of this module error[E0107]: struct takes 0 generic arguments but 1 generic argument was supplied - --> $DIR/multiple-types-with-same-name-and-derive.rs:8:10 + --> $DIR/multiple-types-with-same-name-and-derive.rs:10:10 | LL | #[derive(PartialEq, Eq)] | ^^^^^^^^^ expected 0 generic arguments | note: struct defined here, with 0 generic parameters - --> $DIR/multiple-types-with-same-name-and-derive.rs:6:8 + --> $DIR/multiple-types-with-same-name-and-derive.rs:8:8 | LL | struct NotSM; | ^^^^^ error[E0107]: struct takes 0 generic arguments but 1 generic argument was supplied - --> $DIR/multiple-types-with-same-name-and-derive.rs:8:10 + --> $DIR/multiple-types-with-same-name-and-derive.rs:10:10 | LL | #[derive(PartialEq, Eq)] | ^^^^^^^^^ expected 0 generic arguments | note: struct defined here, with 0 generic parameters - --> $DIR/multiple-types-with-same-name-and-derive.rs:6:8 + --> $DIR/multiple-types-with-same-name-and-derive.rs:8:8 | LL | struct NotSM; | ^^^^^ = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error[E0107]: struct takes 0 generic arguments but 1 generic argument was supplied - --> $DIR/multiple-types-with-same-name-and-derive.rs:8:21 + --> $DIR/multiple-types-with-same-name-and-derive.rs:10:21 | LL | #[derive(PartialEq, Eq)] | ^^ expected 0 generic arguments | note: struct defined here, with 0 generic parameters - --> $DIR/multiple-types-with-same-name-and-derive.rs:6:8 + --> $DIR/multiple-types-with-same-name-and-derive.rs:8:8 | LL | struct NotSM; | ^^^^^ error[E0107]: struct takes 0 generic arguments but 1 generic argument was supplied - --> $DIR/multiple-types-with-same-name-and-derive.rs:8:10 + --> $DIR/multiple-types-with-same-name-and-derive.rs:10:10 | LL | #[derive(PartialEq, Eq)] | ^^^^^^^^^ expected 0 generic arguments | note: struct defined here, with 0 generic parameters - --> $DIR/multiple-types-with-same-name-and-derive.rs:6:8 + --> $DIR/multiple-types-with-same-name-and-derive.rs:8:8 | LL | struct NotSM; | ^^^^^ = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` error[E0609]: no field `0` on type `&NotSM` - --> $DIR/multiple-types-with-same-name-and-derive.rs:13:17 + --> $DIR/multiple-types-with-same-name-and-derive.rs:15:17 | LL | struct NotSM(T); | ^ unknown field