From: Bryysen Date: Sun, 7 Aug 2022 15:24:25 +0000 (+0200) Subject: Further improve error message for E0081 X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=4ee2fe308bc68a0d93a86a09a1fd2506cc41e9b2;p=rust.git Further improve error message for E0081 Multiple duplicate assignments of the same discriminant are now reported in the samme error. We now point out the incrementation start point for discriminants that are not explicitly assigned that are also duplicates. Removed old test related to E0081 that is now covered by error-codes/E0081.rs. Also refactored parts of the `check_enum` function. --- diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs index 9c1fd9b30b4..82d4e235cb3 100644 --- a/compiler/rustc_typeck/src/check/check.rs +++ b/compiler/rustc_typeck/src/check/check.rs @@ -32,7 +32,6 @@ use rustc_trait_selection::traits::{self, ObligationCtxt}; use rustc_ty_utils::representability::{self, Representability}; -use std::iter; use std::ops::ControlFlow; pub(super) fn check_abi(tcx: TyCtxt<'_>, hir_id: hir::HirId, span: Span, abi: Abi) { @@ -1494,76 +1493,96 @@ fn check_enum<'tcx>(tcx: TyCtxt<'tcx>, vs: &'tcx [hir::Variant<'tcx>], def_id: L } } - 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) { - let variant_did = def.variant(VariantIdx::new(i)).def_id; - let variant_i_hir_id = tcx.hir().local_def_id_to_hir_id(variant_did.expect_local()); - let variant_i = tcx.hir().expect_variant(variant_i_hir_id); - let i_span = match variant_i.disr_expr { - Some(ref expr) => tcx.hir().span(expr.hir_id), - None => tcx.def_span(variant_did), - }; - let span = match v.disr_expr { - Some(ref expr) => tcx.hir().span(expr.hir_id), - None => v.span, - }; - 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, - sp, - E0081, - "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; - } + detect_discriminant_duplicate(tcx, def.discriminants(tcx).collect(), vs, sp); check_representable(tcx, sp, def_id); check_transparent(tcx, sp, def); } -/// 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>( +fn detect_discriminant_duplicate<'tcx>( tcx: TyCtxt<'tcx>, - variant: &hir::Variant<'_>, - 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 - && dis.val != *lit_value - { - return format!("`{dis}` (overflowed from `{lit_value}`)"); + mut discrs: Vec<(VariantIdx, Discr<'tcx>)>, + vs: &'tcx [hir::Variant<'tcx>], + self_span: Span, +) { + let report = |var: &hir::Variant<'_>, + dis: Discr<'tcx>, + idx: usize, + err: &mut DiagnosticBuilder<'_, ErrorGuaranteed>| { + let (span, display_discr) = match var.disr_expr { + Some(ref expr) => { + // In the case the discriminant is both a duplicate and overflowed, let the user know + if let hir::ExprKind::Lit(lit) = &tcx.hir().body(expr.body).value.kind + && let rustc_ast::LitKind::Int(lit_value, _int_kind) = &lit.node + && *lit_value != dis.val + { + (tcx.hir().span(expr.hir_id), format!("`{dis}` (overflowed from `{lit_value}`)")) + } else { + (tcx.hir().span(expr.hir_id), format!("`{dis}`")) + } + } + None => { + if let Some((n, hir::Variant { span, ident, .. })) = + vs[..idx].iter().rev().enumerate().find(|v| v.1.disr_expr.is_some()) + { + let ve_ident = var.ident; + let sp = if n > 1 { "variants" } else { "variant" }; + let n = n + 1; + + err.span_label( + *span, + format!("discriminant for `{ve_ident}` incremented from this startpoint (`{ident}` + {n} {sp} later => `{ve_ident}` = {dis})"), + ); + } + + (vs[idx].span, format!("`{dis}`")) + } + }; + + err.span_label(span, format!("{display_discr} assigned here")); + }; + + let mut i = 0; + while i < discrs.len() { + let hir_var_i_idx = discrs[i].0.index(); + let hir_var_i = &vs[hir_var_i_idx]; + let mut error: Option> = None; + + let mut o = i + 1; + while o < discrs.len() { + let hir_var_o_idx = discrs[o].0.index(); + let hir_var_o = &vs[hir_var_o_idx]; + + if discrs[i].1.val == discrs[o].1.val { + let err = error.get_or_insert_with(|| { + let mut ret = struct_span_err!( + tcx.sess, + self_span, + E0081, + "discriminant value `{}` assigned more than once", + discrs[i].1, + ); + + report(hir_var_i, discrs[i].1, hir_var_i_idx, &mut ret); + + ret + }); + + report(hir_var_o, discrs[o].1, hir_var_o_idx, err); + + discrs[o] = *discrs.last().unwrap(); + discrs.pop(); + } else { + o += 1; + } } - } - format!("`{dis}`") + if let Some(mut e) = error { + e.emit(); + } + + i += 1; + } } pub(super) fn check_type_params_are_used<'tcx>( diff --git a/compiler/rustc_typeck/src/check/mod.rs b/compiler/rustc_typeck/src/check/mod.rs index 17c2e4868aa..199277abbc4 100644 --- a/compiler/rustc_typeck/src/check/mod.rs +++ b/compiler/rustc_typeck/src/check/mod.rs @@ -112,7 +112,6 @@ use rustc_hir::intravisit::Visitor; use rustc_hir::{HirIdMap, ImplicitSelfKind, Node}; use rustc_index::bit_set::BitSet; -use rustc_index::vec::Idx; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_middle::ty::query::Providers; use rustc_middle::ty::subst::{InternalSubsts, Subst, SubstsRef}; diff --git a/src/test/ui/enum/enum-discrim-autosizing.rs b/src/test/ui/enum/enum-discrim-autosizing.rs index 27fab1bb505..fc94d281c77 100644 --- a/src/test/ui/enum/enum-discrim-autosizing.rs +++ b/src/test/ui/enum/enum-discrim-autosizing.rs @@ -6,9 +6,9 @@ enum Eu64 { //~^ ERROR discriminant value `0` assigned more than once Au64 = 0, - //~^NOTE first assignment of `0` + //~^NOTE `0` assigned here Bu64 = 0x8000_0000_0000_0000 - //~^NOTE second assignment of `0` (overflowed from `9223372036854775808`) + //~^NOTE `0` (overflowed from `9223372036854775808`) assigned here } fn main() {} diff --git a/src/test/ui/enum/enum-discrim-autosizing.stderr b/src/test/ui/enum/enum-discrim-autosizing.stderr index eacea86d67e..be3d7c64e28 100644 --- a/src/test/ui/enum/enum-discrim-autosizing.stderr +++ b/src/test/ui/enum/enum-discrim-autosizing.stderr @@ -5,10 +5,10 @@ LL | enum Eu64 { | ^^^^^^^^^ LL | LL | Au64 = 0, - | - first assignment of `0` + | - `0` assigned here LL | LL | Bu64 = 0x8000_0000_0000_0000 - | --------------------- second assignment of `0` (overflowed from `9223372036854775808`) + | --------------------- `0` (overflowed from `9223372036854775808`) assigned here 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 5aa6a786339..5e970a89307 100644 --- a/src/test/ui/error-codes/E0081.rs +++ b/src/test/ui/error-codes/E0081.rs @@ -1,9 +1,9 @@ enum Enum { //~^ ERROR discriminant value `3` assigned more than once P = 3, - //~^ NOTE first assignment of `3` + //~^ NOTE `3` assigned here X = 3, - //~^ NOTE second assignment of `3` + //~^ NOTE `3` assigned here Y = 5 } @@ -11,20 +11,38 @@ enum Enum { enum EnumOverflowRepr { //~^ ERROR discriminant value `1` assigned more than once P = 257, - //~^ NOTE first assignment of `1` (overflowed from `257`) + //~^ NOTE `1` (overflowed from `257`) assigned here X = 513, - //~^ NOTE second assignment of `1` (overflowed from `513`) + //~^ NOTE `1` (overflowed from `513`) assigned here } #[repr(i8)] enum NegDisEnum { //~^ ERROR discriminant value `-1` assigned more than once First = -1, - //~^ NOTE first assignment of `-1` + //~^ NOTE `-1` assigned here Second = -2, - //~^ NOTE assigned discriminant for `Last` was incremented from this discriminant + //~^ NOTE discriminant for `Last` incremented from this startpoint (`Second` + 1 variant later => `Last` = -1) Last, - //~^ NOTE second assignment of `-1` + //~^ NOTE `-1` assigned here +} + +#[repr(i32)] +enum MultipleDuplicates { + //~^ ERROR discriminant value `0` assigned more than once + V0, + //~^ NOTE `0` assigned here + V1 = 0, + //~^ NOTE `0` assigned here + V2, + V3, + V4 = 0, + //~^ NOTE `0` assigned here + V5 = -2, + //~^ NOTE discriminant for `V7` incremented from this startpoint (`V5` + 2 variant later => `V7` = 0) + V6, + V7, + //~^ NOTE `0` assigned here } fn main() { diff --git a/src/test/ui/error-codes/E0081.stderr b/src/test/ui/error-codes/E0081.stderr index ff6113646bb..d27861aca5f 100644 --- a/src/test/ui/error-codes/E0081.stderr +++ b/src/test/ui/error-codes/E0081.stderr @@ -5,10 +5,10 @@ LL | enum Enum { | ^^^^^^^^^ LL | LL | P = 3, - | - first assignment of `3` + | - `3` assigned here LL | LL | X = 3, - | - second assignment of `3` + | - `3` assigned here error[E0081]: discriminant value `1` assigned more than once --> $DIR/E0081.rs:11:1 @@ -17,10 +17,10 @@ LL | enum EnumOverflowRepr { | ^^^^^^^^^^^^^^^^^^^^^ LL | LL | P = 257, - | --- first assignment of `1` (overflowed from `257`) + | --- `1` (overflowed from `257`) assigned here LL | LL | X = 513, - | --- second assignment of `1` (overflowed from `513`) + | --- `1` (overflowed from `513`) assigned here error[E0081]: discriminant value `-1` assigned more than once --> $DIR/E0081.rs:20:1 @@ -29,14 +29,35 @@ LL | enum NegDisEnum { | ^^^^^^^^^^^^^^^ LL | LL | First = -1, - | -- first assignment of `-1` + | -- `-1` assigned here LL | LL | Second = -2, - | ----------- assigned discriminant for `Last` was incremented from this discriminant + | ----------- discriminant for `Last` incremented from this startpoint (`Second` + 1 variant later => `Last` = -1) LL | LL | Last, - | ---- second assignment of `-1` + | ---- `-1` assigned here -error: aborting due to 3 previous errors +error[E0081]: discriminant value `0` assigned more than once + --> $DIR/E0081.rs:31:1 + | +LL | enum MultipleDuplicates { + | ^^^^^^^^^^^^^^^^^^^^^^^ +LL | +LL | V0, + | -- `0` assigned here +LL | +LL | V1 = 0, + | - `0` assigned here +... +LL | V4 = 0, + | - `0` assigned here +LL | +LL | V5 = -2, + | ------- discriminant for `V7` incremented from this startpoint (`V5` + 2 variant later => `V7` = 0) +... +LL | V7, + | -- `0` assigned here + +error: aborting due to 4 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 deleted file mode 100644 index 565db2d0fca..00000000000 --- a/src/test/ui/issues/issue-15524.rs +++ /dev/null @@ -1,16 +0,0 @@ -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, - C = 0, - D, - - E = N, - -} - -fn main() {} diff --git a/src/test/ui/issues/issue-15524.stderr b/src/test/ui/issues/issue-15524.stderr deleted file mode 100644 index 1195e0a346d..00000000000 --- a/src/test/ui/issues/issue-15524.stderr +++ /dev/null @@ -1,40 +0,0 @@ -error[E0081]: discriminant value `1` assigned more than once - --> $DIR/issue-15524.rs:3:1 - | -LL | enum Foo { - | ^^^^^^^^ -... -LL | A = 1, - | - first assignment of `1` -LL | B = 1, - | - second assignment of `1` - -error[E0081]: discriminant value `1` assigned more than once - --> $DIR/issue-15524.rs:3:1 - | -LL | enum Foo { - | ^^^^^^^^ -... -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` - -error[E0081]: discriminant value `1` assigned more than once - --> $DIR/issue-15524.rs:3:1 - | -LL | enum Foo { - | ^^^^^^^^ -... -LL | A = 1, - | - first assignment of `1` -... -LL | E = N, - | - second assignment of `1` - -error: aborting due to 3 previous errors - -For more information about this error, try `rustc --explain E0081`. diff --git a/src/test/ui/tag-variant-disr-dup.stderr b/src/test/ui/tag-variant-disr-dup.stderr index 6b1ba43d2ba..4932cde0b18 100644 --- a/src/test/ui/tag-variant-disr-dup.stderr +++ b/src/test/ui/tag-variant-disr-dup.stderr @@ -5,9 +5,9 @@ LL | enum Color { | ^^^^^^^^^^ ... LL | Black = 0x000000, - | -------- first assignment of `0` + | -------- `0` assigned here LL | White = 0x000000, - | -------- second assignment of `0` + | -------- `0` assigned here error: aborting due to previous error