Further improve error message for E0081

Bryysen · Bryysen · commit 4ee2fe308bc6 · 2022-08-07T17:43:52.000+02:00
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
@@ -32,7 +32,6 @@ use rustc_trait_selection::traits::error_reporting::InferCtxtExt as _;
 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<Discr<'tcx>> = 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<DiagnosticBuilder<'_, _>> = 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
@@ -112,7 +112,6 @@ use rustc_hir::def_id::{DefId, LocalDefId};
 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
@@ -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
@@ -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
@@ -1,30 +1,48 @@
 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
 }
 
 #[repr(u8)]
 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
@@ -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
diff --git a/src/test/ui/issues/issue-15524.stderr b/src/test/ui/issues/issue-15524.stderr
diff --git 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
 
