From f671bc969f4bd07490eb30cf230883169924d7a8 Mon Sep 17 00:00:00 2001 From: Bryysen Date: Fri, 27 May 2022 15:42:15 +0200 Subject: [PATCH] Improve error message for E0081 Previously whenever a duplicate discriminant was detected for an Enum, we would print the discriminant bits in the diagnostic without any casting. This caused us to display incorrect values for negative discriminants. After this PR we format the discriminant signedness correctly. Also reworded some of the original error messages. --- compiler/rustc_typeck/src/check/check.rs | 51 ++++++++++----- src/test/ui/enum/enum-discrim-autosizing.rs | 7 +- .../ui/enum/enum-discrim-autosizing.stderr | 18 +++-- src/test/ui/error-codes/E0081.rs | 23 +++++-- src/test/ui/error-codes/E0081.stderr | 58 ++++++++++++----- src/test/ui/issues/issue-15524.rs | 6 +- src/test/ui/issues/issue-15524.stderr | 65 +++++++++++++------ src/test/ui/tag-variant-disr-dup.rs | 3 +- src/test/ui/tag-variant-disr-dup.stderr | 19 ++++-- 9 files changed, 174 insertions(+), 76 deletions(-) diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs index a6d7fecb2e8..09854336599 100644 --- a/compiler/rustc_typeck/src/check/check.rs +++ b/compiler/rustc_typeck/src/check/check.rs @@ -1376,6 +1376,8 @@ fn check_enum<'tcx>( } let mut disr_vals: Vec> = Vec::with_capacity(vs.len()); + // This tracks the previous variant span (in the loop) incase we need it for diagnostics + let mut prev_variant_span: Span = DUMMY_SP; for ((_, discr), v) in iter::zip(def.discriminants(tcx), vs) { // Check for duplicate discriminant values if let Some(i) = disr_vals.iter().position(|&x| x.val == discr.val) { @@ -1390,42 +1392,59 @@ fn check_enum<'tcx>( Some(ref expr) => tcx.hir().span(expr.hir_id), None => v.span, }; - let display_discr = display_discriminant_value(tcx, v, discr.val); - let display_discr_i = display_discriminant_value(tcx, variant_i, disr_vals[i].val); - struct_span_err!( + let display_discr = format_discriminant_overflow(tcx, v, discr); + let display_discr_i = format_discriminant_overflow(tcx, variant_i, disr_vals[i]); + let no_disr = v.disr_expr.is_none(); + let mut err = struct_span_err!( tcx.sess, - span, + sp, E0081, - "discriminant value `{}` already exists", - discr.val, - ) - .span_label(i_span, format!("first use of {display_discr_i}")) - .span_label(span, format!("enum already has {display_discr}")) - .emit(); + "discriminant value `{}` assigned more than once", + discr, + ); + + err.span_label(i_span, format!("first assignment of {display_discr_i}")); + err.span_label(span, format!("second assignment of {display_discr}")); + + if no_disr { + err.span_label( + prev_variant_span, + format!( + "assigned discriminant for `{}` was incremented from this discriminant", + v.ident + ), + ); + } + err.emit(); } + disr_vals.push(discr); + prev_variant_span = v.span; } check_representable(tcx, sp, def_id); check_transparent(tcx, sp, def); } -/// Format an enum discriminant value for use in a diagnostic message. -fn display_discriminant_value<'tcx>( +/// In the case that a discriminant is both a duplicate and an overflowing literal, +/// we insert both the assigned discriminant and the literal it overflowed from into the formatted +/// output. Otherwise we format the discriminant normally. +fn format_discriminant_overflow<'tcx>( tcx: TyCtxt<'tcx>, variant: &hir::Variant<'_>, - evaluated: u128, + dis: Discr<'tcx>, ) -> String { if let Some(expr) = &variant.disr_expr { let body = &tcx.hir().body(expr.body).value; if let hir::ExprKind::Lit(lit) = &body.kind && let rustc_ast::LitKind::Int(lit_value, _int_kind) = &lit.node - && evaluated != *lit_value + && dis.val != *lit_value { - return format!("`{evaluated}` (overflowed from `{lit_value}`)"); + return format!("`{dis}` (overflowed from `{lit_value}`)"); } } - format!("`{}`", evaluated) + + format!("`{dis}`") } pub(super) fn check_type_params_are_used<'tcx>( diff --git a/src/test/ui/enum/enum-discrim-autosizing.rs b/src/test/ui/enum/enum-discrim-autosizing.rs index 473a0ad6f7a..27fab1bb505 100644 --- a/src/test/ui/enum/enum-discrim-autosizing.rs +++ b/src/test/ui/enum/enum-discrim-autosizing.rs @@ -4,10 +4,11 @@ // so force the repr. #[cfg_attr(not(target_pointer_width = "32"), repr(i32))] enum Eu64 { - Au64 = 0, //~NOTE first use of `0` + //~^ ERROR discriminant value `0` assigned more than once + Au64 = 0, + //~^NOTE first assignment of `0` Bu64 = 0x8000_0000_0000_0000 - //~^ ERROR discriminant value `0` already exists - //~| NOTE enum already has `0` (overflowed from `9223372036854775808`) + //~^NOTE second assignment of `0` (overflowed from `9223372036854775808`) } fn main() {} diff --git a/src/test/ui/enum/enum-discrim-autosizing.stderr b/src/test/ui/enum/enum-discrim-autosizing.stderr index a0f39098a2e..bcd362b0632 100644 --- a/src/test/ui/enum/enum-discrim-autosizing.stderr +++ b/src/test/ui/enum/enum-discrim-autosizing.stderr @@ -1,10 +1,16 @@ -error[E0081]: discriminant value `0` already exists - --> $DIR/enum-discrim-autosizing.rs:8:12 +error[E0081]: discriminant value `0` assigned more than once + --> $DIR/enum-discrim-autosizing.rs:6:1 | -LL | Au64 = 0, - | - first use of `0` -LL | Bu64 = 0x8000_0000_0000_0000 - | ^^^^^^^^^^^^^^^^^^^^^ enum already has `0` (overflowed from `9223372036854775808`) +LL | / enum Eu64 { +LL | | +LL | | Au64 = 0, + | | - first assignment of `0` +LL | | +LL | | Bu64 = 0x8000_0000_0000_0000 + | | --------------------- second assignment of `0` (overflowed from `9223372036854775808`) +LL | | +LL | | } + | |_^ error: aborting due to previous error diff --git a/src/test/ui/error-codes/E0081.rs b/src/test/ui/error-codes/E0081.rs index 255e05ced19..5aa6a786339 100644 --- a/src/test/ui/error-codes/E0081.rs +++ b/src/test/ui/error-codes/E0081.rs @@ -1,19 +1,30 @@ enum Enum { + //~^ ERROR discriminant value `3` assigned more than once P = 3, - //~^ NOTE first use of `3` + //~^ NOTE first assignment of `3` X = 3, - //~^ ERROR discriminant value `3` already exists - //~| NOTE enum already has `3` + //~^ NOTE second assignment of `3` Y = 5 } #[repr(u8)] enum EnumOverflowRepr { + //~^ ERROR discriminant value `1` assigned more than once P = 257, - //~^ NOTE first use of `1` (overflowed from `257`) + //~^ NOTE first assignment of `1` (overflowed from `257`) X = 513, - //~^ ERROR discriminant value `1` already exists - //~| NOTE enum already has `1` (overflowed from `513`) + //~^ NOTE second assignment of `1` (overflowed from `513`) +} + +#[repr(i8)] +enum NegDisEnum { + //~^ ERROR discriminant value `-1` assigned more than once + First = -1, + //~^ NOTE first assignment of `-1` + Second = -2, + //~^ NOTE assigned discriminant for `Last` was incremented from this discriminant + Last, + //~^ NOTE second assignment of `-1` } fn main() { diff --git a/src/test/ui/error-codes/E0081.stderr b/src/test/ui/error-codes/E0081.stderr index 9b279bb0214..d7c7dbac7d6 100644 --- a/src/test/ui/error-codes/E0081.stderr +++ b/src/test/ui/error-codes/E0081.stderr @@ -1,21 +1,49 @@ -error[E0081]: discriminant value `3` already exists - --> $DIR/E0081.rs:4:9 +error[E0081]: discriminant value `3` assigned more than once + --> $DIR/E0081.rs:1:1 | -LL | P = 3, - | - first use of `3` -LL | -LL | X = 3, - | ^ enum already has `3` +LL | / enum Enum { +LL | | +LL | | P = 3, + | | - first assignment of `3` +LL | | +LL | | X = 3, + | | - second assignment of `3` +LL | | +LL | | Y = 5 +LL | | } + | |_^ -error[E0081]: discriminant value `1` already exists - --> $DIR/E0081.rs:14:9 +error[E0081]: discriminant value `1` assigned more than once + --> $DIR/E0081.rs:11:1 | -LL | P = 257, - | --- first use of `1` (overflowed from `257`) -LL | -LL | X = 513, - | ^^^ enum already has `1` (overflowed from `513`) +LL | / enum EnumOverflowRepr { +LL | | +LL | | P = 257, + | | --- first assignment of `1` (overflowed from `257`) +LL | | +LL | | X = 513, + | | --- second assignment of `1` (overflowed from `513`) +LL | | +LL | | } + | |_^ -error: aborting due to 2 previous errors +error[E0081]: discriminant value `-1` assigned more than once + --> $DIR/E0081.rs:20:1 + | +LL | / enum NegDisEnum { +LL | | +LL | | First = -1, + | | -- first assignment of `-1` +LL | | +LL | | Second = -2, + | | ----------- assigned discriminant for `Last` was incremented from this discriminant +LL | | +LL | | Last, + | | ---- second assignment of `-1` +LL | | +LL | | } + | |_^ + +error: aborting due to 3 previous errors For more information about this error, try `rustc --explain E0081`. diff --git a/src/test/ui/issues/issue-15524.rs b/src/test/ui/issues/issue-15524.rs index eb7ec419c2e..565db2d0fca 100644 --- a/src/test/ui/issues/issue-15524.rs +++ b/src/test/ui/issues/issue-15524.rs @@ -1,15 +1,15 @@ const N: isize = 1; enum Foo { + //~^ ERROR discriminant value `1` assigned more than once + //~| ERROR discriminant value `1` assigned more than once + //~| ERROR discriminant value `1` assigned more than once A = 1, B = 1, - //~^ ERROR discriminant value `1` already exists C = 0, D, - //~^ ERROR discriminant value `1` already exists E = N, - //~^ ERROR discriminant value `1` already exists } diff --git a/src/test/ui/issues/issue-15524.stderr b/src/test/ui/issues/issue-15524.stderr index 1702dad34ca..a0ea0c2459f 100644 --- a/src/test/ui/issues/issue-15524.stderr +++ b/src/test/ui/issues/issue-15524.stderr @@ -1,28 +1,53 @@ -error[E0081]: discriminant value `1` already exists - --> $DIR/issue-15524.rs:5:9 +error[E0081]: discriminant value `1` assigned more than once + --> $DIR/issue-15524.rs:3:1 | -LL | A = 1, - | - first use of `1` -LL | B = 1, - | ^ enum already has `1` +LL | / enum Foo { +LL | | +LL | | +LL | | +LL | | A = 1, + | | - first assignment of `1` +LL | | B = 1, + | | - second assignment of `1` +... | +LL | | +LL | | } + | |_^ -error[E0081]: discriminant value `1` already exists - --> $DIR/issue-15524.rs:8:5 +error[E0081]: discriminant value `1` assigned more than once + --> $DIR/issue-15524.rs:3:1 | -LL | A = 1, - | - first use of `1` -... -LL | D, - | ^ enum already has `1` +LL | / enum Foo { +LL | | +LL | | +LL | | +LL | | A = 1, + | | - first assignment of `1` +LL | | B = 1, +LL | | C = 0, + | | ----- assigned discriminant for `D` was incremented from this discriminant +LL | | D, + | | - second assignment of `1` +... | +LL | | +LL | | } + | |_^ -error[E0081]: discriminant value `1` already exists - --> $DIR/issue-15524.rs:11:9 +error[E0081]: discriminant value `1` assigned more than once + --> $DIR/issue-15524.rs:3:1 | -LL | A = 1, - | - first use of `1` -... -LL | E = N, - | ^ enum already has `1` +LL | / enum Foo { +LL | | +LL | | +LL | | +LL | | A = 1, + | | - first assignment of `1` +... | +LL | | E = N, + | | - second assignment of `1` +LL | | +LL | | } + | |_^ error: aborting due to 3 previous errors diff --git a/src/test/ui/tag-variant-disr-dup.rs b/src/test/ui/tag-variant-disr-dup.rs index d2a38e003ea..e497f993da2 100644 --- a/src/test/ui/tag-variant-disr-dup.rs +++ b/src/test/ui/tag-variant-disr-dup.rs @@ -1,11 +1,12 @@ // Black and White have the same discriminator value ... enum Color { + //~^ ERROR discriminant value `0` assigned more than once Red = 0xff0000, Green = 0x00ff00, Blue = 0x0000ff, Black = 0x000000, - White = 0x000000, //~ ERROR discriminant value `0` already exists + White = 0x000000, } fn main() { } diff --git a/src/test/ui/tag-variant-disr-dup.stderr b/src/test/ui/tag-variant-disr-dup.stderr index ca12894c042..27adb6998ae 100644 --- a/src/test/ui/tag-variant-disr-dup.stderr +++ b/src/test/ui/tag-variant-disr-dup.stderr @@ -1,10 +1,17 @@ -error[E0081]: discriminant value `0` already exists - --> $DIR/tag-variant-disr-dup.rs:8:13 +error[E0081]: discriminant value `0` assigned more than once + --> $DIR/tag-variant-disr-dup.rs:3:1 | -LL | Black = 0x000000, - | -------- first use of `0` -LL | White = 0x000000, - | ^^^^^^^^ enum already has `0` +LL | / enum Color { +LL | | +LL | | Red = 0xff0000, +LL | | Green = 0x00ff00, +LL | | Blue = 0x0000ff, +LL | | Black = 0x000000, + | | -------- first assignment of `0` +LL | | White = 0x000000, + | | -------- second assignment of `0` +LL | | } + | |_^ error: aborting due to previous error