From abc1ad71062dd8e2b7ae97ada736f2d0c0b2344e Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 30 Oct 2022 18:31:03 +0000 Subject: [PATCH] Use AdtDef to check enum. --- .../rustc_hir_analysis/src/check/check.rs | 121 +++++++++--------- src/test/ui/error-codes/E0081.stderr | 6 +- 2 files changed, 65 insertions(+), 62 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index b70ac02058d3..133bbd52b914 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -6,7 +6,7 @@ use super::*; use rustc_attr as attr; use rustc_errors::{Applicability, ErrorGuaranteed, MultiSpan}; use rustc_hir as hir; -use rustc_hir::def::{DefKind, Res}; +use rustc_hir::def::{CtorKind, DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::intravisit::Visitor; use rustc_hir::{ItemKind, Node, PathSegment}; @@ -75,7 +75,7 @@ fn check_struct(tcx: TyCtxt<'_>, def_id: LocalDefId) { check_simd(tcx, span, def_id); } - check_transparent(tcx, span, def); + check_transparent(tcx, def); check_packed(tcx, span, def); } @@ -83,7 +83,7 @@ fn check_union(tcx: TyCtxt<'_>, def_id: LocalDefId) { let def = tcx.adt_def(def_id); let span = tcx.def_span(def_id); def.destructor(tcx); // force the destructor to be evaluated - check_transparent(tcx, span, def); + check_transparent(tcx, def); check_union_fields(tcx, span, def_id); check_packed(tcx, span, def); } @@ -506,11 +506,7 @@ fn check_item_type<'tcx>(tcx: TyCtxt<'tcx>, id: hir::ItemId) { tcx.ensure().typeck(id.owner_id.def_id); } DefKind::Enum => { - let item = tcx.hir().item(id); - let hir::ItemKind::Enum(ref enum_definition, _) = item.kind else { - return; - }; - check_enum(tcx, &enum_definition.variants, item.owner_id.def_id); + check_enum(tcx, id.owner_id.def_id); } DefKind::Fn => {} // entirely within check_item_body DefKind::Impl => { @@ -1026,7 +1022,7 @@ pub(super) fn check_packed_inner( None } -pub(super) fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, adt: ty::AdtDef<'tcx>) { +pub(super) fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, adt: ty::AdtDef<'tcx>) { if !adt.repr().transparent() { return; } @@ -1035,14 +1031,14 @@ pub(super) fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, adt: ty::AdtD feature_err( &tcx.sess.parse_sess, sym::transparent_unions, - sp, + tcx.def_span(adt.did()), "transparent unions are unstable", ) .emit(); } if adt.variants().len() != 1 { - bad_variant_count(tcx, adt, sp, adt.did()); + bad_variant_count(tcx, adt, tcx.def_span(adt.did()), adt.did()); if adt.variants().is_empty() { // Don't bother checking the fields. No variants (and thus no fields) exist. return; @@ -1103,7 +1099,7 @@ pub(super) fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, adt: ty::AdtD .filter_map(|(span, zst, _align1, _non_exhaustive)| if !zst { Some(span) } else { None }); let non_zst_count = non_zst_fields.clone().count(); if non_zst_count >= 2 { - bad_non_zero_sized_fields(tcx, adt, non_zst_count, non_zst_fields, sp); + bad_non_zero_sized_fields(tcx, adt, non_zst_count, non_zst_fields, tcx.def_span(adt.did())); } let incompatible_zst_fields = field_infos.clone().filter(|(_, _, _, opt)| opt.is_some()).count(); @@ -1143,12 +1139,11 @@ pub(super) fn check_transparent<'tcx>(tcx: TyCtxt<'tcx>, sp: Span, adt: ty::AdtD } #[allow(trivial_numeric_casts)] -fn check_enum<'tcx>(tcx: TyCtxt<'tcx>, vs: &'tcx [hir::Variant<'tcx>], def_id: LocalDefId) { +fn check_enum<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) { let def = tcx.adt_def(def_id); - let sp = tcx.def_span(def_id); def.destructor(tcx); // force the destructor to be evaluated - if vs.is_empty() { + if def.variants().is_empty() { if let Some(attr) = tcx.get_attrs(def_id.to_def_id(), sym::repr).next() { struct_span_err!( tcx.sess, @@ -1156,7 +1151,7 @@ fn check_enum<'tcx>(tcx: TyCtxt<'tcx>, vs: &'tcx [hir::Variant<'tcx>], def_id: L E0084, "unsupported representation for zero-variant enum" ) - .span_label(sp, "zero-variant enum") + .span_label(tcx.def_span(def_id), "zero-variant enum") .emit(); } } @@ -1167,88 +1162,96 @@ fn check_enum<'tcx>(tcx: TyCtxt<'tcx>, vs: &'tcx [hir::Variant<'tcx>], def_id: L feature_err( &tcx.sess.parse_sess, sym::repr128, - sp, + tcx.def_span(def_id), "repr with 128-bit type is unstable", ) .emit(); } } - for v in vs { - if let Some(ref e) = v.disr_expr { - tcx.ensure().typeck(tcx.hir().local_def_id(e.hir_id)); + for v in def.variants() { + if let ty::VariantDiscr::Explicit(discr_def_id) = v.discr { + tcx.ensure().typeck(discr_def_id.expect_local()); } } - if tcx.adt_def(def_id).repr().int.is_none() { - let is_unit = |var: &hir::Variant<'_>| matches!(var.data, hir::VariantData::Unit(..)); + if def.repr().int.is_none() { + let is_unit = |var: &ty::VariantDef| matches!(var.ctor_kind, CtorKind::Const); + let has_disr = |var: &ty::VariantDef| matches!(var.discr, ty::VariantDiscr::Explicit(_)); - let has_disr = |var: &hir::Variant<'_>| var.disr_expr.is_some(); - let has_non_units = vs.iter().any(|var| !is_unit(var)); - let disr_units = vs.iter().any(|var| is_unit(&var) && has_disr(&var)); - let disr_non_unit = vs.iter().any(|var| !is_unit(&var) && has_disr(&var)); + let has_non_units = def.variants().iter().any(|var| !is_unit(var)); + let disr_units = def.variants().iter().any(|var| is_unit(&var) && has_disr(&var)); + let disr_non_unit = def.variants().iter().any(|var| !is_unit(&var) && has_disr(&var)); if disr_non_unit || (disr_units && has_non_units) { - let mut err = - struct_span_err!(tcx.sess, sp, E0732, "`#[repr(inttype)]` must be specified"); + let mut err = struct_span_err!( + tcx.sess, + tcx.def_span(def_id), + E0732, + "`#[repr(inttype)]` must be specified" + ); err.emit(); } } - detect_discriminant_duplicate(tcx, def.discriminants(tcx).collect(), vs, sp); - - check_transparent(tcx, sp, def); + detect_discriminant_duplicate(tcx, def); + check_transparent(tcx, def); } /// Part of enum check. Given the discriminants of an enum, errors if two or more discriminants are equal -fn detect_discriminant_duplicate<'tcx>( - tcx: TyCtxt<'tcx>, - mut discrs: Vec<(VariantIdx, Discr<'tcx>)>, - vs: &'tcx [hir::Variant<'tcx>], - self_span: Span, -) { +fn detect_discriminant_duplicate<'tcx>(tcx: TyCtxt<'tcx>, adt: ty::AdtDef<'tcx>) { // Helper closure to reduce duplicate code. This gets called everytime we detect a duplicate. // Here `idx` refers to the order of which the discriminant appears, and its index in `vs` - let report = |dis: Discr<'tcx>, idx: usize, err: &mut Diagnostic| { - let var = &vs[idx]; // HIR for the duplicate discriminant - let (span, display_discr) = match var.disr_expr { - Some(ref expr) => { + let report = |dis: Discr<'tcx>, idx, err: &mut Diagnostic| { + let var = adt.variant(idx); // HIR for the duplicate discriminant + let (span, display_discr) = match var.discr { + ty::VariantDiscr::Explicit(discr_def_id) => { // 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 + if let hir::Node::AnonConst(expr) = tcx.hir().get_by_def_id(discr_def_id.expect_local()) + && 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}`)")) - // Otherwise, format the value as-is + (tcx.def_span(discr_def_id), format!("`{dis}` (overflowed from `{lit_value}`)")) } else { - (tcx.hir().span(expr.hir_id), format!("`{dis}`")) + // Otherwise, format the value as-is + (tcx.def_span(discr_def_id), format!("`{dis}`")) } } - None => { + // This should not happen. + ty::VariantDiscr::Relative(0) => (tcx.def_span(var.def_id), format!("`{dis}`")), + ty::VariantDiscr::Relative(distance_to_explicit) => { // At this point we know this discriminant is a duplicate, and was not explicitly // assigned by the user. Here we iterate backwards to fetch the HIR for the last // explicitly assigned discriminant, and letting the user know that this was the // increment startpoint, and how many steps from there leading to the duplicate - if let Some((n, hir::Variant { span, ident, .. })) = - vs[..idx].iter().rev().enumerate().find(|v| v.1.disr_expr.is_some()) + if let Some(explicit_idx) = + idx.as_u32().checked_sub(distance_to_explicit).map(VariantIdx::from_u32) { - let ve_ident = var.ident; - let n = n + 1; - let sp = if n > 1 { "variants" } else { "variant" }; + let explicit_variant = adt.variant(explicit_idx); + let ve_ident = var.name; + let ex_ident = explicit_variant.name; + let sp = if distance_to_explicit > 1 { "variants" } else { "variant" }; err.span_label( - *span, - format!("discriminant for `{ve_ident}` incremented from this startpoint (`{ident}` + {n} {sp} later => `{ve_ident}` = {dis})"), + tcx.def_span(explicit_variant.def_id), + format!( + "discriminant for `{ve_ident}` incremented from this startpoint \ + (`{ex_ident}` + {distance_to_explicit} {sp} later \ + => `{ve_ident}` = {dis})" + ), ); } - (vs[idx].span, format!("`{dis}`")) + (tcx.def_span(var.def_id), format!("`{dis}`")) } }; err.span_label(span, format!("{display_discr} assigned here")); }; + let mut discrs = adt.discriminants(tcx).collect::>(); + // Here we loop through the discriminants, comparing each discriminant to another. // When a duplicate is detected, we instantiate an error and point to both // initial and duplicate value. The duplicate discriminant is then discarded by swapping @@ -1257,29 +1260,29 @@ fn detect_discriminant_duplicate<'tcx>( // style as we are mutating `discrs` on the fly). let mut i = 0; while i < discrs.len() { - let hir_var_i_idx = discrs[i].0.index(); + let var_i_idx = discrs[i].0; let mut error: Option> = None; let mut o = i + 1; while o < discrs.len() { - let hir_var_o_idx = discrs[o].0.index(); + let var_o_idx = discrs[o].0; 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, + tcx.def_span(adt.did()), E0081, "discriminant value `{}` assigned more than once", discrs[i].1, ); - report(discrs[i].1, hir_var_i_idx, &mut ret); + report(discrs[i].1, var_i_idx, &mut ret); ret }); - report(discrs[o].1, hir_var_o_idx, err); + report(discrs[o].1, var_o_idx, err); // Safe to unwrap here, as we wouldn't reach this point if `discrs` was empty discrs[o] = *discrs.last().unwrap(); diff --git a/src/test/ui/error-codes/E0081.stderr b/src/test/ui/error-codes/E0081.stderr index 64562fefc866..d4b21f6893b4 100644 --- a/src/test/ui/error-codes/E0081.stderr +++ b/src/test/ui/error-codes/E0081.stderr @@ -32,7 +32,7 @@ LL | First = -1, | -- `-1` assigned here LL | LL | Second = -2, - | ----------- discriminant for `Last` incremented from this startpoint (`Second` + 1 variant later => `Last` = -1) + | ------ discriminant for `Last` incremented from this startpoint (`Second` + 1 variant later => `Last` = -1) LL | LL | Last, | ---- `-1` assigned here @@ -53,7 +53,7 @@ LL | V4 = 0, | - `0` assigned here LL | LL | V5 = -2, - | ------- discriminant for `V7` incremented from this startpoint (`V5` + 2 variants later => `V7` = 0) + | -- discriminant for `V7` incremented from this startpoint (`V5` + 2 variants later => `V7` = 0) ... LL | V7, | -- `0` assigned here @@ -68,7 +68,7 @@ LL | V5 = -2, | -- `-2` assigned here ... LL | V8 = -3, - | ------- discriminant for `V9` incremented from this startpoint (`V8` + 1 variant later => `V9` = -2) + | -- discriminant for `V9` incremented from this startpoint (`V8` + 1 variant later => `V9` = -2) LL | LL | V9, | -- `-2` assigned here