From 3dfd0fd85840bd60fb2956297d6073d791b81d4d Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 17 Mar 2024 15:48:27 +0100 Subject: [PATCH 01/12] Report arm intersections --- .../rustc_pattern_analysis/src/usefulness.rs | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/usefulness.rs b/compiler/rustc_pattern_analysis/src/usefulness.rs index 3760db8b68894..cdc03eaeb37c1 100644 --- a/compiler/rustc_pattern_analysis/src/usefulness.rs +++ b/compiler/rustc_pattern_analysis/src/usefulness.rs @@ -1042,7 +1042,7 @@ struct MatrixRow<'p, Cx: PatCx> { is_under_guard: bool, /// When we specialize, we remember which row of the original matrix produced a given row of the /// specialized matrix. When we unspecialize, we use this to propagate usefulness back up the - /// callstack. + /// callstack. On creation, this stores the index of the original match arm. parent_row: usize, /// False when the matrix is just built. This is set to `true` by /// [`compute_exhaustiveness_and_usefulness`] if the arm is found to be useful. @@ -1163,10 +1163,10 @@ impl<'p, Cx: PatCx> Matrix<'p, Cx> { place_info: smallvec![place_info], wildcard_row_is_relevant: true, }; - for (row_id, arm) in arms.iter().enumerate() { + for (arm_id, arm) in arms.iter().enumerate() { let v = MatrixRow { pats: PatStack::from_pattern(arm.pat), - parent_row: row_id, // dummy, we don't read it + parent_row: arm_id, is_under_guard: arm.has_guard, useful: false, intersects: BitSet::new_empty(0), // Initialized in `Matrix::expand_and_push`. @@ -1738,6 +1738,9 @@ pub struct UsefulnessReport<'p, Cx: PatCx> { /// If the match is exhaustive, this is empty. If not, this contains witnesses for the lack of /// exhaustiveness. pub non_exhaustiveness_witnesses: Vec>, + /// For each arm, a set of indices of arms above it that have non-empty intersection, i.e. there + /// is a value matched by both arms. This may miss real intersections. + pub arm_intersections: Vec>, } /// Computes whether a match is exhaustive and which of its arms are useful. @@ -1769,5 +1772,19 @@ pub fn compute_match_usefulness<'p, Cx: PatCx>( }) .collect(); - Ok(UsefulnessReport { arm_usefulness, non_exhaustiveness_witnesses }) + let mut arm_intersections: Vec<_> = + arms.iter().enumerate().map(|(i, _)| BitSet::new_empty(i)).collect(); + for row in matrix.rows() { + let arm_id = row.parent_row; + for intersection in row.intersects.iter() { + // Convert the matrix row ids into arm ids (they can differ because we expand or-patterns). + let arm_intersection = matrix.rows[intersection].parent_row; + // Note: self-intersection can happen with or-patterns. + if arm_intersection != arm_id { + arm_intersections[arm_id].insert(arm_intersection); + } + } + } + + Ok(UsefulnessReport { arm_usefulness, non_exhaustiveness_witnesses, arm_intersections }) } From e4487ad391503fa109506c40aeba377fda0d97dd Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 5 Mar 2024 00:16:11 +0100 Subject: [PATCH 02/12] Improve the `WitnessPat: Debug` impl --- .../rustc_pattern_analysis/src/constructor.rs | 75 +++++++++++++++++ compiler/rustc_pattern_analysis/src/lib.rs | 3 +- compiler/rustc_pattern_analysis/src/pat.rs | 80 ++----------------- compiler/rustc_pattern_analysis/src/rustc.rs | 7 +- 4 files changed, 89 insertions(+), 76 deletions(-) diff --git a/compiler/rustc_pattern_analysis/src/constructor.rs b/compiler/rustc_pattern_analysis/src/constructor.rs index 5b58d7f43b2b7..95c5556410d34 100644 --- a/compiler/rustc_pattern_analysis/src/constructor.rs +++ b/compiler/rustc_pattern_analysis/src/constructor.rs @@ -819,6 +819,81 @@ impl Constructor { } }) } + + pub(crate) fn fmt_fields( + &self, + f: &mut fmt::Formatter<'_>, + ty: &Cx::Ty, + mut fields: impl Iterator, + ) -> fmt::Result { + let mut first = true; + let mut start_or_continue = |s| { + if first { + first = false; + "" + } else { + s + } + }; + let mut start_or_comma = || start_or_continue(", "); + + match self { + Struct | Variant(_) | UnionField => { + Cx::write_variant_name(f, self, ty)?; + // Without `cx`, we can't know which field corresponds to which, so we can't + // get the names of the fields. Instead we just display everything as a tuple + // struct, which should be good enough. + write!(f, "(")?; + for p in fields { + write!(f, "{}{:?}", start_or_comma(), p)?; + } + write!(f, ")")?; + } + // Note: given the expansion of `&str` patterns done in `expand_pattern`, we should + // be careful to detect strings here. However a string literal pattern will never + // be reported as a non-exhaustiveness witness, so we can ignore this issue. + Ref => { + write!(f, "&{:?}", &fields.next().unwrap())?; + } + Slice(slice) => { + write!(f, "[")?; + match slice.kind { + SliceKind::FixedLen(_) => { + for p in fields { + write!(f, "{}{:?}", start_or_comma(), p)?; + } + } + SliceKind::VarLen(prefix_len, _) => { + for p in fields.by_ref().take(prefix_len) { + write!(f, "{}{:?}", start_or_comma(), p)?; + } + write!(f, "{}..", start_or_comma())?; + for p in fields { + write!(f, "{}{:?}", start_or_comma(), p)?; + } + } + } + write!(f, "]")?; + } + Bool(b) => write!(f, "{b}")?, + // Best-effort, will render signed ranges incorrectly + IntRange(range) => write!(f, "{range:?}")?, + F32Range(lo, hi, end) => write!(f, "{lo}{end}{hi}")?, + F64Range(lo, hi, end) => write!(f, "{lo}{end}{hi}")?, + Str(value) => write!(f, "{value:?}")?, + Opaque(..) => write!(f, "")?, + Or => { + for pat in fields { + write!(f, "{}{:?}", start_or_continue(" | "), pat)?; + } + } + Never => write!(f, "!")?, + Wildcard | Missing | NonExhaustive | Hidden | PrivateUninhabited => { + write!(f, "_ : {:?}", ty)? + } + } + Ok(()) + } } #[derive(Debug, Clone, Copy)] diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index 5c57c990323c8..3904959fa0f5b 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -120,7 +120,8 @@ pub trait PatCx: Sized + fmt::Debug { /// `DeconstructedPat`. Only invoqued when `pat.ctor()` is `Struct | Variant(_) | UnionField`. fn write_variant_name( f: &mut fmt::Formatter<'_>, - pat: &crate::pat::DeconstructedPat, + ctor: &crate::constructor::Constructor, + ty: &Self::Ty, ) -> fmt::Result; /// Raise a bug. diff --git a/compiler/rustc_pattern_analysis/src/pat.rs b/compiler/rustc_pattern_analysis/src/pat.rs index e7eed673c9441..5f388ee9f8979 100644 --- a/compiler/rustc_pattern_analysis/src/pat.rs +++ b/compiler/rustc_pattern_analysis/src/pat.rs @@ -138,81 +138,11 @@ impl DeconstructedPat { /// This is best effort and not good enough for a `Display` impl. impl fmt::Debug for DeconstructedPat { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let pat = self; - let mut first = true; - let mut start_or_continue = |s| { - if first { - first = false; - "" - } else { - s - } - }; - let mut start_or_comma = || start_or_continue(", "); - let mut fields: Vec<_> = (0..self.arity).map(|_| PatOrWild::Wild).collect(); for ipat in self.iter_fields() { fields[ipat.idx] = PatOrWild::Pat(&ipat.pat); } - - match pat.ctor() { - Struct | Variant(_) | UnionField => { - Cx::write_variant_name(f, pat)?; - // Without `cx`, we can't know which field corresponds to which, so we can't - // get the names of the fields. Instead we just display everything as a tuple - // struct, which should be good enough. - write!(f, "(")?; - for p in fields { - write!(f, "{}", start_or_comma())?; - write!(f, "{p:?}")?; - } - write!(f, ")") - } - // Note: given the expansion of `&str` patterns done in `expand_pattern`, we should - // be careful to detect strings here. However a string literal pattern will never - // be reported as a non-exhaustiveness witness, so we can ignore this issue. - Ref => { - write!(f, "&{:?}", &fields[0]) - } - Slice(slice) => { - write!(f, "[")?; - match slice.kind { - SliceKind::FixedLen(_) => { - for p in fields { - write!(f, "{}{:?}", start_or_comma(), p)?; - } - } - SliceKind::VarLen(prefix_len, _) => { - for p in &fields[..prefix_len] { - write!(f, "{}{:?}", start_or_comma(), p)?; - } - write!(f, "{}", start_or_comma())?; - write!(f, "..")?; - for p in &fields[prefix_len..] { - write!(f, "{}{:?}", start_or_comma(), p)?; - } - } - } - write!(f, "]") - } - Bool(b) => write!(f, "{b}"), - // Best-effort, will render signed ranges incorrectly - IntRange(range) => write!(f, "{range:?}"), - F32Range(lo, hi, end) => write!(f, "{lo}{end}{hi}"), - F64Range(lo, hi, end) => write!(f, "{lo}{end}{hi}"), - Str(value) => write!(f, "{value:?}"), - Opaque(..) => write!(f, ""), - Or => { - for pat in fields { - write!(f, "{}{:?}", start_or_continue(" | "), pat)?; - } - Ok(()) - } - Never => write!(f, "!"), - Wildcard | Missing | NonExhaustive | Hidden | PrivateUninhabited => { - write!(f, "_ : {:?}", pat.ty()) - } - } + self.ctor().fmt_fields(f, self.ty(), fields.into_iter()) } } @@ -295,7 +225,6 @@ impl<'p, Cx: PatCx> fmt::Debug for PatOrWild<'p, Cx> { /// Same idea as `DeconstructedPat`, except this is a fictitious pattern built up for diagnostics /// purposes. As such they don't use interning and can be cloned. -#[derive(Debug)] pub struct WitnessPat { ctor: Constructor, pub(crate) fields: Vec>, @@ -353,3 +282,10 @@ impl WitnessPat { self.fields.iter() } } + +/// This is best effort and not good enough for a `Display` impl. +impl fmt::Debug for WitnessPat { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.ctor().fmt_fields(f, self.ty(), self.fields.iter()) + } +} diff --git a/compiler/rustc_pattern_analysis/src/rustc.rs b/compiler/rustc_pattern_analysis/src/rustc.rs index eedc00a561304..f600c20d38470 100644 --- a/compiler/rustc_pattern_analysis/src/rustc.rs +++ b/compiler/rustc_pattern_analysis/src/rustc.rs @@ -874,13 +874,14 @@ impl<'p, 'tcx: 'p> PatCx for RustcPatCtxt<'p, 'tcx> { fn write_variant_name( f: &mut fmt::Formatter<'_>, - pat: &crate::pat::DeconstructedPat, + ctor: &crate::constructor::Constructor, + ty: &Self::Ty, ) -> fmt::Result { - if let ty::Adt(adt, _) = pat.ty().kind() { + if let ty::Adt(adt, _) = ty.kind() { if adt.is_box() { write!(f, "Box")? } else { - let variant = adt.variant(Self::variant_index_for_adt(pat.ctor(), *adt)); + let variant = adt.variant(Self::variant_index_for_adt(ctor, *adt)); write!(f, "{}", variant.name)?; } } From d697dd44d18fbab9a28032d7b1ceba829600637e Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Mon, 4 Mar 2024 16:57:32 +0100 Subject: [PATCH 03/12] Add a crate-custom test harness --- Cargo.lock | 2 + compiler/rustc_pattern_analysis/Cargo.toml | 4 + compiler/rustc_pattern_analysis/src/lib.rs | 6 + .../tests/common/mod.rs | 315 ++++++++++++++++++ .../tests/complexity.rs | 109 ++++++ .../tests/exhaustiveness.rs | 77 +++++ .../tests/intersection.rs | 69 ++++ 7 files changed, 582 insertions(+) create mode 100644 compiler/rustc_pattern_analysis/tests/common/mod.rs create mode 100644 compiler/rustc_pattern_analysis/tests/complexity.rs create mode 100644 compiler/rustc_pattern_analysis/tests/exhaustiveness.rs create mode 100644 compiler/rustc_pattern_analysis/tests/intersection.rs diff --git a/Cargo.lock b/Cargo.lock index 3110f32ade968..0b9eb88ec2b39 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4440,6 +4440,8 @@ dependencies = [ "rustc_target", "smallvec", "tracing", + "tracing-subscriber", + "tracing-tree", ] [[package]] diff --git a/compiler/rustc_pattern_analysis/Cargo.toml b/compiler/rustc_pattern_analysis/Cargo.toml index b9bdcb41929dc..6357d18b9da84 100644 --- a/compiler/rustc_pattern_analysis/Cargo.toml +++ b/compiler/rustc_pattern_analysis/Cargo.toml @@ -22,6 +22,10 @@ smallvec = { version = "1.8.1", features = ["union"] } tracing = "0.1" # tidy-alphabetical-end +[dev-dependencies] +tracing-subscriber = { version = "0.3.3", default-features = false, features = ["fmt", "env-filter", "ansi"] } +tracing-tree = "0.2.0" + [features] default = ["rustc"] rustc = [ diff --git a/compiler/rustc_pattern_analysis/src/lib.rs b/compiler/rustc_pattern_analysis/src/lib.rs index 3904959fa0f5b..1a1da5c55f608 100644 --- a/compiler/rustc_pattern_analysis/src/lib.rs +++ b/compiler/rustc_pattern_analysis/src/lib.rs @@ -49,6 +49,12 @@ pub mod index { } } + impl FromIterator for IdxContainer { + fn from_iter>(iter: T) -> Self { + Self(iter.into_iter().enumerate().collect()) + } + } + #[derive(Debug)] pub struct IdxSet(pub rustc_hash::FxHashSet); impl IdxSet { diff --git a/compiler/rustc_pattern_analysis/tests/common/mod.rs b/compiler/rustc_pattern_analysis/tests/common/mod.rs new file mode 100644 index 0000000000000..e72fddb9e9a81 --- /dev/null +++ b/compiler/rustc_pattern_analysis/tests/common/mod.rs @@ -0,0 +1,315 @@ +use rustc_pattern_analysis::{ + constructor::{ + Constructor, ConstructorSet, IntRange, MaybeInfiniteInt, RangeEnd, VariantVisibility, + }, + usefulness::{PlaceValidity, UsefulnessReport}, + Captures, MatchArm, PatCx, PrivateUninhabitedField, +}; + +/// Sets up `tracing` for easier debugging. Tries to look like the `rustc` setup. +pub fn init_tracing() { + use tracing_subscriber::layer::SubscriberExt; + use tracing_subscriber::util::SubscriberInitExt; + use tracing_subscriber::Layer; + let _ = tracing_tree::HierarchicalLayer::default() + .with_writer(std::io::stderr) + .with_indent_lines(true) + .with_ansi(true) + .with_targets(true) + .with_indent_amount(2) + .with_subscriber( + tracing_subscriber::Registry::default() + .with(tracing_subscriber::EnvFilter::from_default_env()), + ) + .try_init(); +} + +/// A simple set of types. +#[allow(dead_code)] +#[derive(Debug, Copy, Clone)] +pub enum Ty { + /// Booleans + Bool, + /// 8-bit unsigned integers + U8, + /// Tuples. + Tuple(&'static [Ty]), + /// A struct with `arity` fields of type `ty`. + BigStruct { arity: usize, ty: &'static Ty }, + /// A enum with `arity` variants of type `ty`. + BigEnum { arity: usize, ty: &'static Ty }, +} + +/// The important logic. +impl Ty { + pub fn sub_tys(&self, ctor: &Constructor) -> Vec { + use Constructor::*; + match (ctor, *self) { + (Struct, Ty::Tuple(tys)) => tys.iter().copied().collect(), + (Struct, Ty::BigStruct { arity, ty }) => (0..arity).map(|_| *ty).collect(), + (Variant(_), Ty::BigEnum { ty, .. }) => vec![*ty], + (Bool(..) | IntRange(..) | NonExhaustive | Missing | Wildcard, _) => vec![], + _ => panic!("Unexpected ctor {ctor:?} for type {self:?}"), + } + } + + pub fn ctor_set(&self) -> ConstructorSet { + match *self { + Ty::Bool => ConstructorSet::Bool, + Ty::U8 => ConstructorSet::Integers { + range_1: IntRange::from_range( + MaybeInfiniteInt::new_finite_uint(0), + MaybeInfiniteInt::new_finite_uint(255), + RangeEnd::Included, + ), + range_2: None, + }, + Ty::Tuple(..) | Ty::BigStruct { .. } => ConstructorSet::Struct { empty: false }, + Ty::BigEnum { arity, .. } => ConstructorSet::Variants { + variants: (0..arity).map(|_| VariantVisibility::Visible).collect(), + non_exhaustive: false, + }, + } + } + + pub fn write_variant_name( + &self, + f: &mut std::fmt::Formatter<'_>, + ctor: &Constructor, + ) -> std::fmt::Result { + match (*self, ctor) { + (Ty::Tuple(..), _) => Ok(()), + (Ty::BigStruct { .. }, _) => write!(f, "BigStruct"), + (Ty::BigEnum { .. }, Constructor::Variant(i)) => write!(f, "BigEnum::Variant{i}"), + _ => write!(f, "{:?}::{:?}", self, ctor), + } + } +} + +/// Compute usefulness in our simple context (and set up tracing for easier debugging). +pub fn compute_match_usefulness<'p>( + arms: &[MatchArm<'p, Cx>], + ty: Ty, + scrut_validity: PlaceValidity, + complexity_limit: Option, +) -> Result, ()> { + init_tracing(); + rustc_pattern_analysis::usefulness::compute_match_usefulness( + &Cx, + arms, + ty, + scrut_validity, + complexity_limit, + ) +} + +#[derive(Debug)] +pub struct Cx; + +/// The context for pattern analysis. Forwards anything interesting to `Ty` methods. +impl PatCx for Cx { + type Ty = Ty; + type Error = (); + type VariantIdx = usize; + type StrLit = (); + type ArmData = (); + type PatData = (); + + fn is_exhaustive_patterns_feature_on(&self) -> bool { + false + } + + fn is_min_exhaustive_patterns_feature_on(&self) -> bool { + false + } + + fn ctor_arity(&self, ctor: &Constructor, ty: &Self::Ty) -> usize { + ty.sub_tys(ctor).len() + } + + fn ctor_sub_tys<'a>( + &'a self, + ctor: &'a Constructor, + ty: &'a Self::Ty, + ) -> impl Iterator + ExactSizeIterator + Captures<'a> + { + ty.sub_tys(ctor).into_iter().map(|ty| (ty, PrivateUninhabitedField(false))) + } + + fn ctors_for_ty(&self, ty: &Self::Ty) -> Result, Self::Error> { + Ok(ty.ctor_set()) + } + + fn write_variant_name( + f: &mut std::fmt::Formatter<'_>, + ctor: &Constructor, + ty: &Self::Ty, + ) -> std::fmt::Result { + ty.write_variant_name(f, ctor) + } + + fn bug(&self, fmt: std::fmt::Arguments<'_>) -> Self::Error { + panic!("{}", fmt) + } + + /// Abort when reaching the complexity limit. This is what we'll check in tests. + fn complexity_exceeded(&self) -> Result<(), Self::Error> { + Err(()) + } +} + +/// Construct a single pattern; see `pats!()`. +#[allow(unused_macros)] +macro_rules! pat { + ($($rest:tt)*) => {{ + let mut vec = pats!($($rest)*); + vec.pop().unwrap() + }}; +} + +/// A macro to construct patterns. Called like `pats!(type_expr; pattern, pattern, ..)` and returns +/// a `Vec`. A pattern can be nested and looks like `Constructor(pat, pat)` or +/// `Constructor { .i: pat, .j: pat }`, where `Constructor` is `Struct`, `Variant.i` (with index +/// `i`), as well as booleans and integer ranges. +/// +/// The general structure of the macro is a tt-muncher with several stages identified with +/// `@something(args)`. The args are a key-value list (the keys ensure we don't mix the arguments +/// around) which is passed down and modified as needed. We then parse token-trees from +/// left-to-right. Non-trivial recursion happens when we parse the arguments to a pattern: we +/// recurse to parse the tokens inside `{..}`/`(..)`, and then we continue parsing anything that +/// follows. +macro_rules! pats { + // Entrypoint + // Parse `type; ..` + ($ty:expr; $($rest:tt)*) => {{ + #[allow(unused_imports)] + use rustc_pattern_analysis::{ + constructor::{Constructor, IntRange, MaybeInfiniteInt, RangeEnd}, + pat::DeconstructedPat, + }; + let ty = $ty; + // The heart of the macro is designed to push `IndexedPat`s into a `Vec`, so we work around + // that. + let sub_tys = ::std::iter::repeat(&ty); + let mut vec = Vec::new(); + pats!(@ctor(vec:vec, sub_tys:sub_tys, idx:0) $($rest)*); + vec.into_iter().map(|ipat| ipat.pat).collect::>() + }}; + + // Parse `constructor ..` + + (@ctor($($args:tt)*) true $($rest:tt)*) => {{ + let ctor = Constructor::Bool(true); + pats!(@pat($($args)*, ctor:ctor) $($rest)*) + }}; + (@ctor($($args:tt)*) false $($rest:tt)*) => {{ + let ctor = Constructor::Bool(false); + pats!(@pat($($args)*, ctor:ctor) $($rest)*) + }}; + (@ctor($($args:tt)*) Struct $($rest:tt)*) => {{ + let ctor = Constructor::Struct; + pats!(@pat($($args)*, ctor:ctor) $($rest)*) + }}; + (@ctor($($args:tt)*) ( $($fields:tt)* ) $($rest:tt)*) => {{ + let ctor = Constructor::Struct; // tuples + pats!(@pat($($args)*, ctor:ctor) ( $($fields)* ) $($rest)*) + }}; + (@ctor($($args:tt)*) Variant.$variant:ident $($rest:tt)*) => {{ + let ctor = Constructor::Variant($variant); + pats!(@pat($($args)*, ctor:ctor) $($rest)*) + }}; + (@ctor($($args:tt)*) Variant.$variant:literal $($rest:tt)*) => {{ + let ctor = Constructor::Variant($variant); + pats!(@pat($($args)*, ctor:ctor) $($rest)*) + }}; + (@ctor($($args:tt)*) _ $($rest:tt)*) => {{ + let ctor = Constructor::Wildcard; + pats!(@pat($($args)*, ctor:ctor) $($rest)*) + }}; + + // Integers and int ranges + (@ctor($($args:tt)*) $($start:literal)?..$end:literal $($rest:tt)*) => {{ + let ctor = Constructor::IntRange(IntRange::from_range( + pats!(@rangeboundary- $($start)?), + pats!(@rangeboundary+ $end), + RangeEnd::Excluded, + )); + pats!(@pat($($args)*, ctor:ctor) $($rest)*) + }}; + (@ctor($($args:tt)*) $($start:literal)?.. $($rest:tt)*) => {{ + let ctor = Constructor::IntRange(IntRange::from_range( + pats!(@rangeboundary- $($start)?), + pats!(@rangeboundary+), + RangeEnd::Excluded, + )); + pats!(@pat($($args)*, ctor:ctor) $($rest)*) + }}; + (@ctor($($args:tt)*) $($start:literal)?..=$end:literal $($rest:tt)*) => {{ + let ctor = Constructor::IntRange(IntRange::from_range( + pats!(@rangeboundary- $($start)?), + pats!(@rangeboundary+ $end), + RangeEnd::Included, + )); + pats!(@pat($($args)*, ctor:ctor) $($rest)*) + }}; + (@ctor($($args:tt)*) $int:literal $($rest:tt)*) => {{ + let ctor = Constructor::IntRange(IntRange::from_range( + pats!(@rangeboundary- $int), + pats!(@rangeboundary+ $int), + RangeEnd::Included, + )); + pats!(@pat($($args)*, ctor:ctor) $($rest)*) + }}; + // Utility to manage range boundaries. + (@rangeboundary $sign:tt $int:literal) => { MaybeInfiniteInt::new_finite_uint($int) }; + (@rangeboundary -) => { MaybeInfiniteInt::NegInfinity }; + (@rangeboundary +) => { MaybeInfiniteInt::PosInfinity }; + + // Parse subfields: `(..)` or `{..}` + + // Constructor with no fields, e.g. `bool` or `Variant.1`. + (@pat($($args:tt)*) $(,)?) => { + pats!(@pat($($args)*) {}) + }; + (@pat($($args:tt)*) , $($rest:tt)*) => { + pats!(@pat($($args)*) {}, $($rest)*) + }; + // `(..)` and `{..}` are treated the same. + (@pat($($args:tt)*) ( $($subpat:tt)* ) $($rest:tt)*) => {{ + pats!(@pat($($args)*) { $($subpat)* } $($rest)*) + }}; + (@pat(vec:$vec:expr, sub_tys:$sub_tys:expr, idx:$idx:expr, ctor:$ctor:expr) { $($fields:tt)* } $($rest:tt)*) => {{ + let sub_tys = $sub_tys; + let index = $idx; + // Silly dance to work with both a vec and `iter::repeat()`. + let ty = *(&sub_tys).clone().into_iter().nth(index).unwrap(); + let ctor = $ctor; + let ctor_sub_tys = &ty.sub_tys(&ctor); + #[allow(unused_mut)] + let mut fields = Vec::new(); + // Parse subpatterns (note the leading comma). + pats!(@fields(idx:0, vec:fields, sub_tys:ctor_sub_tys) ,$($fields)*); + let arity = ctor_sub_tys.len(); + let pat = DeconstructedPat::new(ctor, fields, arity, ty, ()).at_index(index); + $vec.push(pat); + + // Continue parsing further patterns. + pats!(@fields(idx:index+1, vec:$vec, sub_tys:sub_tys) $($rest)*); + }}; + + // Parse fields one by one. + + // No fields left. + (@fields($($args:tt)*) $(,)?) => {}; + // `.i: pat` sets the current index to `i`. + (@fields(idx:$_idx:expr, $($args:tt)*) , .$idx:literal : $($rest:tt)*) => {{ + pats!(@ctor($($args)*, idx:$idx) $($rest)*); + }}; + (@fields(idx:$_idx:expr, $($args:tt)*) , .$idx:ident : $($rest:tt)*) => {{ + pats!(@ctor($($args)*, idx:$idx) $($rest)*); + }}; + // Field without an explicit index; we use the current index which gets incremented above. + (@fields(idx:$idx:expr, $($args:tt)*) , $($rest:tt)*) => {{ + pats!(@ctor($($args)*, idx:$idx) $($rest)*); + }}; +} diff --git a/compiler/rustc_pattern_analysis/tests/complexity.rs b/compiler/rustc_pattern_analysis/tests/complexity.rs new file mode 100644 index 0000000000000..93f455c6257d2 --- /dev/null +++ b/compiler/rustc_pattern_analysis/tests/complexity.rs @@ -0,0 +1,109 @@ +//! Test the pattern complexity limit. +use common::*; +use rustc_pattern_analysis::{pat::DeconstructedPat, usefulness::PlaceValidity, MatchArm}; + +#[macro_use] +mod common; + +/// Analyze a match made of these patterns. Ignore the report; we only care whether we exceeded the +/// limit or not. +fn check(patterns: &[DeconstructedPat], complexity_limit: usize) -> Result<(), ()> { + let ty = *patterns[0].ty(); + let arms: Vec<_> = + patterns.iter().map(|pat| MatchArm { pat, has_guard: false, arm_data: () }).collect(); + compute_match_usefulness(arms.as_slice(), ty, PlaceValidity::ValidOnly, Some(complexity_limit)) + .map(|_report| ()) +} + +/// Asserts that analyzing this match takes exactly `complexity` steps. +#[track_caller] +fn assert_complexity(patterns: Vec>, complexity: usize) { + assert!(check(&patterns, complexity).is_ok()); + assert!(check(&patterns, complexity - 1).is_err()); +} + +/// Construct a match like: +/// ```ignore(illustrative) +/// match ... { +/// BigStruct { field01: true, .. } => {} +/// BigStruct { field02: true, .. } => {} +/// BigStruct { field03: true, .. } => {} +/// BigStruct { field04: true, .. } => {} +/// ... +/// _ => {} +/// } +/// ``` +fn diagonal_match(arity: usize) -> Vec> { + let struct_ty = Ty::BigStruct { arity, ty: &Ty::Bool }; + let mut patterns = vec![]; + for i in 0..arity { + patterns.push(pat!(struct_ty; Struct { .i: true })); + } + patterns.push(pat!(struct_ty; _)); + patterns +} + +/// Construct a match like: +/// ```ignore(illustrative) +/// match ... { +/// BigStruct { field01: true, .. } => {} +/// BigStruct { field02: true, .. } => {} +/// BigStruct { field03: true, .. } => {} +/// BigStruct { field04: true, .. } => {} +/// ... +/// BigStruct { field01: false, .. } => {} +/// BigStruct { field02: false, .. } => {} +/// BigStruct { field03: false, .. } => {} +/// BigStruct { field04: false, .. } => {} +/// ... +/// _ => {} +/// } +/// ``` +fn diagonal_exponential_match(arity: usize) -> Vec> { + let struct_ty = Ty::BigStruct { arity, ty: &Ty::Bool }; + let mut patterns = vec![]; + for i in 0..arity { + patterns.push(pat!(struct_ty; Struct { .i: true })); + } + for i in 0..arity { + patterns.push(pat!(struct_ty; Struct { .i: false })); + } + patterns.push(pat!(struct_ty; _)); + patterns +} + +#[test] +fn test_diagonal_struct_match() { + // These cases are nicely linear: we check `arity` patterns with exactly one `true`, matching + // in 2 branches each, and a final pattern with all `false`, matching only the `_` branch. + assert_complexity(diagonal_match(20), 41); + assert_complexity(diagonal_match(30), 61); + // This case goes exponential. + assert!(check(&diagonal_exponential_match(10), 10000).is_err()); +} + +/// Construct a match like: +/// ```ignore(illustrative) +/// match ... { +/// BigEnum::Variant1(_) => {} +/// BigEnum::Variant2(_) => {} +/// BigEnum::Variant3(_) => {} +/// ... +/// _ => {} +/// } +/// ``` +fn big_enum(arity: usize) -> Vec> { + let enum_ty = Ty::BigEnum { arity, ty: &Ty::Bool }; + let mut patterns = vec![]; + for i in 0..arity { + patterns.push(pat!(enum_ty; Variant.i)); + } + patterns.push(pat!(enum_ty; _)); + patterns +} + +#[test] +fn test_big_enum() { + // We try 2 branches per variant. + assert_complexity(big_enum(20), 40); +} diff --git a/compiler/rustc_pattern_analysis/tests/exhaustiveness.rs b/compiler/rustc_pattern_analysis/tests/exhaustiveness.rs new file mode 100644 index 0000000000000..4c6c72fa8ecbc --- /dev/null +++ b/compiler/rustc_pattern_analysis/tests/exhaustiveness.rs @@ -0,0 +1,77 @@ +//! Test exhaustiveness checking. +use common::*; +use rustc_pattern_analysis::{ + pat::{DeconstructedPat, WitnessPat}, + usefulness::PlaceValidity, + MatchArm, +}; + +#[macro_use] +mod common; + +/// Analyze a match made of these patterns. +fn check(patterns: Vec>) -> Vec> { + let ty = *patterns[0].ty(); + let arms: Vec<_> = + patterns.iter().map(|pat| MatchArm { pat, has_guard: false, arm_data: () }).collect(); + let report = + compute_match_usefulness(arms.as_slice(), ty, PlaceValidity::ValidOnly, None).unwrap(); + report.non_exhaustiveness_witnesses +} + +#[track_caller] +fn assert_exhaustive(patterns: Vec>) { + let witnesses = check(patterns); + if !witnesses.is_empty() { + panic!("non-exaustive match: missing {witnesses:?}"); + } +} + +#[track_caller] +fn assert_non_exhaustive(patterns: Vec>) { + let witnesses = check(patterns); + assert!(!witnesses.is_empty()) +} + +#[test] +fn test_int_ranges() { + let ty = Ty::U8; + assert_exhaustive(pats!(ty; + 0..=255, + )); + assert_exhaustive(pats!(ty; + 0.., + )); + assert_non_exhaustive(pats!(ty; + 0..255, + )); + assert_exhaustive(pats!(ty; + 0..255, + 255, + )); + assert_exhaustive(pats!(ty; + ..10, + 10.. + )); +} + +#[test] +fn test_nested() { + let ty = Ty::BigStruct { arity: 2, ty: &Ty::BigEnum { arity: 2, ty: &Ty::Bool } }; + assert_non_exhaustive(pats!(ty; + Struct(Variant.0, _), + )); + assert_exhaustive(pats!(ty; + Struct(Variant.0, _), + Struct(Variant.1, _), + )); + assert_non_exhaustive(pats!(ty; + Struct(Variant.0, _), + Struct(_, Variant.0), + )); + assert_exhaustive(pats!(ty; + Struct(Variant.0, _), + Struct(_, Variant.0), + Struct(Variant.1, Variant.1), + )); +} diff --git a/compiler/rustc_pattern_analysis/tests/intersection.rs b/compiler/rustc_pattern_analysis/tests/intersection.rs new file mode 100644 index 0000000000000..4d8a21506d760 --- /dev/null +++ b/compiler/rustc_pattern_analysis/tests/intersection.rs @@ -0,0 +1,69 @@ +//! Test the computation of arm intersections. +use common::*; +use rustc_pattern_analysis::{pat::DeconstructedPat, usefulness::PlaceValidity, MatchArm}; + +#[macro_use] +mod common; + +/// Analyze a match made of these patterns and returns the computed arm intersections. +fn check(patterns: Vec>) -> Vec> { + let ty = *patterns[0].ty(); + let arms: Vec<_> = + patterns.iter().map(|pat| MatchArm { pat, has_guard: false, arm_data: () }).collect(); + let report = + compute_match_usefulness(arms.as_slice(), ty, PlaceValidity::ValidOnly, None).unwrap(); + report.arm_intersections.into_iter().map(|bitset| bitset.iter().collect()).collect() +} + +#[track_caller] +fn assert_intersects(patterns: Vec>, intersects: &[&[usize]]) { + let computed_intersects = check(patterns); + assert_eq!(computed_intersects, intersects); +} + +#[test] +fn test_int_ranges() { + let ty = Ty::U8; + assert_intersects( + pats!(ty; + 0..=100, + 100.., + ), + &[&[], &[0]], + ); + assert_intersects( + pats!(ty; + 0..=101, + 100.., + ), + &[&[], &[0]], + ); + assert_intersects( + pats!(ty; + 0..100, + 100.., + ), + &[&[], &[]], + ); +} + +#[test] +fn test_nested() { + let ty = Ty::Tuple(&[Ty::Bool; 2]); + assert_intersects( + pats!(ty; + (true, true), + (true, _), + (_, true), + ), + &[&[], &[0], &[0, 1]], + ); + // Here we shortcut because `(true, true)` is irrelevant, so we fail to detect the intersection. + assert_intersects( + pats!(ty; + (true, _), + (_, true), + ), + &[&[], &[]], + ); +} From 45cb9d8ec3419bbf411c8fe49639b84040fc9d23 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 19 Mar 2024 10:04:32 +0000 Subject: [PATCH 04/12] Add regression test for #107495 --- .../issue-107495-archive-permissions/foo.rs | 1 + .../issue-107495-archive-permissions/rmake.rs | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 tests/run-make/issue-107495-archive-permissions/foo.rs create mode 100644 tests/run-make/issue-107495-archive-permissions/rmake.rs diff --git a/tests/run-make/issue-107495-archive-permissions/foo.rs b/tests/run-make/issue-107495-archive-permissions/foo.rs new file mode 100644 index 0000000000000..8b137891791fe --- /dev/null +++ b/tests/run-make/issue-107495-archive-permissions/foo.rs @@ -0,0 +1 @@ + diff --git a/tests/run-make/issue-107495-archive-permissions/rmake.rs b/tests/run-make/issue-107495-archive-permissions/rmake.rs new file mode 100644 index 0000000000000..593ba4b4ae70f --- /dev/null +++ b/tests/run-make/issue-107495-archive-permissions/rmake.rs @@ -0,0 +1,22 @@ +extern crate run_make_support; + +use run_make_support::{aux_build, out_dir}; +use std::fs; +#[cfg(unix)] +use std::os::unix::fs::PermissionsExt; +use std::path::Path; + +fn main() { + aux_build().arg("foo.rs").run(); + verify(&out_dir().join("libfoo.rlib")); +} + +fn verify(path: &Path) { + let perm = fs::metadata(path).unwrap().permissions(); + + assert!(!perm.readonly()); + + // Check that the file is readable for everyone + #[cfg(unix)] + assert_eq!(perm.mode(), 0o100664); +} From 2a805f59defa34c04206b74aaa483e8ea0b9dc93 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 19 Mar 2024 10:10:47 +0000 Subject: [PATCH 05/12] Use the default file permissions when writing static libraries with ar_archive_writer Fixes #107495 --- .../rustc_codegen_ssa/src/back/archive.rs | 37 +++++++++++-------- .../issue-107495-archive-permissions/foo.rs | 2 +- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/archive.rs b/compiler/rustc_codegen_ssa/src/back/archive.rs index 22b58c13949e2..bb285a3bbd02c 100644 --- a/compiler/rustc_codegen_ssa/src/back/archive.rs +++ b/compiler/rustc_codegen_ssa/src/back/archive.rs @@ -13,7 +13,7 @@ use object::read::macho::FatArch; use tempfile::Builder as TempFileBuilder; use std::error::Error; -use std::fs::File; +use std::fs::{self, File}; use std::io::{self, Write}; use std::path::{Path, PathBuf}; @@ -276,19 +276,22 @@ impl<'a> ArArchiveBuilder<'a> { // This prevents programs (including rustc) from attempting to read a partial archive. // It also enables writing an archive with the same filename as a dependency on Windows as // required by a test. - let mut archive_tmpfile = TempFileBuilder::new() + // The tempfile crate currently uses 0o600 as mode for the temporary files and directories + // it creates. We need it to be the default mode for back compat reasons however. (See + // #107495) To handle this we are telling tempfile to create a temporary directory instead + // and then inside this directory create a file using File::create. + let archive_tmpdir = TempFileBuilder::new() .suffix(".temp-archive") - .tempfile_in(output.parent().unwrap_or_else(|| Path::new(""))) - .map_err(|err| io_error_context("couldn't create a temp file", err))?; - - write_archive_to_stream( - archive_tmpfile.as_file_mut(), - &entries, - true, - archive_kind, - true, - false, - )?; + .tempdir_in(output.parent().unwrap_or_else(|| Path::new(""))) + .map_err(|err| { + io_error_context("couldn't create a directory for the temp file", err) + })?; + let archive_tmpfile_path = archive_tmpdir.path().join("tmp.a"); + let mut archive_tmpfile = File::create_new(&archive_tmpfile_path) + .map_err(|err| io_error_context("couldn't create the temp file", err))?; + + write_archive_to_stream(&mut archive_tmpfile, &entries, true, archive_kind, true, false)?; + drop(archive_tmpfile); let any_entries = !entries.is_empty(); drop(entries); @@ -296,9 +299,11 @@ impl<'a> ArArchiveBuilder<'a> { // output archive to the same location as an input archive on Windows. drop(self.src_archives); - archive_tmpfile - .persist(output) - .map_err(|err| io_error_context("failed to rename archive file", err.error))?; + fs::rename(archive_tmpfile_path, output) + .map_err(|err| io_error_context("failed to rename archive file", err))?; + archive_tmpdir + .close() + .map_err(|err| io_error_context("failed to remove temporary directory", err))?; Ok(any_entries) } diff --git a/tests/run-make/issue-107495-archive-permissions/foo.rs b/tests/run-make/issue-107495-archive-permissions/foo.rs index 8b137891791fe..d15abba59766b 100644 --- a/tests/run-make/issue-107495-archive-permissions/foo.rs +++ b/tests/run-make/issue-107495-archive-permissions/foo.rs @@ -1 +1 @@ - +// Empty From 75a5196490fc1d8e0e39b406ce99a30d115bcfc5 Mon Sep 17 00:00:00 2001 From: Tshepang Mbambo Date: Tue, 19 Mar 2024 20:32:55 +0200 Subject: [PATCH 06/12] use more accurate terminology rustc is just one tool/executable, even if at the center of the toolchain --- config.example.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.example.toml b/config.example.toml index f94553dd63f72..b8cdc2ec84804 100644 --- a/config.example.toml +++ b/config.example.toml @@ -915,6 +915,6 @@ # Available options: fast, balanced, best #compression-profile = "fast" -# Copy the linker, DLLs, and various libraries from MinGW into the rustc toolchain. +# Copy the linker, DLLs, and various libraries from MinGW into the Rust toolchain. # Only applies when the host or target is pc-windows-gnu. #include-mingw-linker = true From 37718f949fe5bef4bbfa4ee393c2abf022771ccb Mon Sep 17 00:00:00 2001 From: Joshua Wong Date: Tue, 19 Mar 2024 22:47:35 -0500 Subject: [PATCH 07/12] fix OOB pointer formed in Vec::index Move the length check to before using `index` with `ptr::add` to prevent an out of bounds pointer from being formed. Fixes #122760 --- library/alloc/src/vec/mod.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/library/alloc/src/vec/mod.rs b/library/alloc/src/vec/mod.rs index db56b8d07c71f..94bed825bb2f6 100644 --- a/library/alloc/src/vec/mod.rs +++ b/library/alloc/src/vec/mod.rs @@ -1541,6 +1541,9 @@ impl Vec { } let len = self.len(); + if index > len { + assert_failed(index, len); + } // space for the new element if len == self.buf.capacity() { @@ -1556,10 +1559,6 @@ impl Vec { // Shift everything over to make space. (Duplicating the // `index`th element into two consecutive places.) ptr::copy(p, p.add(1), len - index); - } else if index == len { - // No elements need shifting. - } else { - assert_failed(index, len); } // Write it in, overwriting the first copy of the `index`th // element. From 70206f06ca5cff08d3b84240074ec89e705ab61b Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 20 Mar 2024 18:00:12 +1100 Subject: [PATCH 08/12] coverage: Regression test for ICE triggered by self-loops --- tests/coverage/let_else_loop.cov-map | 30 +++++++++++++++++++++++ tests/coverage/let_else_loop.coverage | 35 +++++++++++++++++++++++++++ tests/coverage/let_else_loop.rs | 33 +++++++++++++++++++++++++ 3 files changed, 98 insertions(+) create mode 100644 tests/coverage/let_else_loop.cov-map create mode 100644 tests/coverage/let_else_loop.coverage create mode 100644 tests/coverage/let_else_loop.rs diff --git a/tests/coverage/let_else_loop.cov-map b/tests/coverage/let_else_loop.cov-map new file mode 100644 index 0000000000000..b0cee30052200 --- /dev/null +++ b/tests/coverage/let_else_loop.cov-map @@ -0,0 +1,30 @@ +Function name: let_else_loop::_if (unused) +Raw bytes (19): 0x[01, 01, 00, 03, 00, 16, 01, 01, 0c, 00, 02, 09, 00, 10, 00, 02, 09, 00, 10] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 3 +- Code(Zero) at (prev + 22, 1) to (start + 1, 12) +- Code(Zero) at (prev + 2, 9) to (start + 0, 16) +- Code(Zero) at (prev + 2, 9) to (start + 0, 16) + +Function name: let_else_loop::_loop_either_way (unused) +Raw bytes (19): 0x[01, 01, 00, 03, 00, 0f, 01, 01, 14, 00, 01, 1c, 00, 23, 00, 01, 05, 00, 0c] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 3 +- Code(Zero) at (prev + 15, 1) to (start + 1, 20) +- Code(Zero) at (prev + 1, 28) to (start + 0, 35) +- Code(Zero) at (prev + 1, 5) to (start + 0, 12) + +Function name: let_else_loop::loopy +Raw bytes (19): 0x[01, 01, 00, 03, 01, 09, 01, 01, 14, 00, 01, 1c, 00, 23, 05, 01, 01, 00, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 3 +- Code(Counter(0)) at (prev + 9, 1) to (start + 1, 20) +- Code(Zero) at (prev + 1, 28) to (start + 0, 35) +- Code(Counter(1)) at (prev + 1, 1) to (start + 0, 2) + diff --git a/tests/coverage/let_else_loop.coverage b/tests/coverage/let_else_loop.coverage new file mode 100644 index 0000000000000..3b3c39ff30180 --- /dev/null +++ b/tests/coverage/let_else_loop.coverage @@ -0,0 +1,35 @@ + LL| |#![feature(coverage_attribute)] + LL| |//@ edition: 2021 + LL| |//@ ignore-test + LL| |// Regression test for . + LL| |// These code patterns should not trigger an ICE when allocating a physical + LL| |// counter to a node and also one of its in-edges, because that is allowed + LL| |// when the node contains a tight loop to itself. + LL| | + LL| 1|fn loopy(cond: bool) { + LL| 1| let true = cond else { loop {} }; + ^0 + LL| 1|} + LL| | + LL| |// Variant that also has `loop {}` on the success path. + LL| |// This isn't needed to catch the original ICE, but might help detect regressions. + LL| 0|fn _loop_either_way(cond: bool) { + LL| 0| let true = cond else { loop {} }; + LL| 0| loop {} + LL| |} + LL| | + LL| |// Variant using regular `if` instead of let-else. + LL| |// This doesn't trigger the original ICE, but might help detect regressions. + LL| 0|fn _if(cond: bool) { + LL| 0| if cond { + LL| 0| loop {} + LL| | } else { + LL| 0| loop {} + LL| | } + LL| |} + LL| | + LL| |#[coverage(off)] + LL| |fn main() { + LL| | loopy(true); + LL| |} + diff --git a/tests/coverage/let_else_loop.rs b/tests/coverage/let_else_loop.rs new file mode 100644 index 0000000000000..92a252cbbdc99 --- /dev/null +++ b/tests/coverage/let_else_loop.rs @@ -0,0 +1,33 @@ +#![feature(coverage_attribute)] +//@ edition: 2021 +//@ ignore-test +// Regression test for . +// These code patterns should not trigger an ICE when allocating a physical +// counter to a node and also one of its in-edges, because that is allowed +// when the node contains a tight loop to itself. + +fn loopy(cond: bool) { + let true = cond else { loop {} }; +} + +// Variant that also has `loop {}` on the success path. +// This isn't needed to catch the original ICE, but might help detect regressions. +fn _loop_either_way(cond: bool) { + let true = cond else { loop {} }; + loop {} +} + +// Variant using regular `if` instead of let-else. +// This doesn't trigger the original ICE, but might help detect regressions. +fn _if(cond: bool) { + if cond { + loop {} + } else { + loop {} + } +} + +#[coverage(off)] +fn main() { + loopy(true); +} From 85bec7a50c9a8ab466f8fbafa7b0c4eb05e6c721 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 20 Mar 2024 18:17:23 +1100 Subject: [PATCH 09/12] coverage: Remove incorrect assertions from counter allocation These assertions detect situations where a BCB node would have both a physical counter and one or more in-edge counters/expressions. For most BCBs that situation would indicate an implementation bug. However, it's perfectly fine in the case of a BCB having an edge that loops back to itself. Given the complexity and risk involved in fixing the assertions, and the fact that nothing relies on them actually being true, this patch just removes them instead. --- .../src/coverage/counters.rs | 31 ------------------- tests/coverage/let_else_loop.coverage | 2 +- tests/coverage/let_else_loop.rs | 2 +- 3 files changed, 2 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index 9a1d8bae6b410..402ce6ba8d9a9 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -1,7 +1,6 @@ use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::graph::WithNumNodes; -use rustc_index::bit_set::BitSet; use rustc_index::IndexVec; use rustc_middle::mir::coverage::*; @@ -18,10 +17,6 @@ pub(super) enum BcbCounter { } impl BcbCounter { - fn is_expression(&self) -> bool { - matches!(self, Self::Expression { .. }) - } - pub(super) fn as_term(&self) -> CovTerm { match *self { BcbCounter::Counter { id, .. } => CovTerm::Counter(id), @@ -60,10 +55,6 @@ pub(super) struct CoverageCounters { /// We currently don't iterate over this map, but if we do in the future, /// switch it back to `FxIndexMap` to avoid query stability hazards. bcb_edge_counters: FxHashMap<(BasicCoverageBlock, BasicCoverageBlock), BcbCounter>, - /// Tracks which BCBs have a counter associated with some incoming edge. - /// Only used by assertions, to verify that BCBs with incoming edge - /// counters do not have their own physical counters (expressions are allowed). - bcb_has_incoming_edge_counters: BitSet, /// Table of expression data, associating each expression ID with its /// corresponding operator (+ or -) and its LHS/RHS operands. expressions: IndexVec, @@ -83,7 +74,6 @@ impl CoverageCounters { counter_increment_sites: IndexVec::new(), bcb_counters: IndexVec::from_elem_n(None, num_bcbs), bcb_edge_counters: FxHashMap::default(), - bcb_has_incoming_edge_counters: BitSet::new_empty(num_bcbs), expressions: IndexVec::new(), }; @@ -122,14 +112,6 @@ impl CoverageCounters { } fn set_bcb_counter(&mut self, bcb: BasicCoverageBlock, counter_kind: BcbCounter) -> BcbCounter { - assert!( - // If the BCB has an edge counter (to be injected into a new `BasicBlock`), it can also - // have an expression (to be injected into an existing `BasicBlock` represented by this - // `BasicCoverageBlock`). - counter_kind.is_expression() || !self.bcb_has_incoming_edge_counters.contains(bcb), - "attempt to add a `Counter` to a BCB target with existing incoming edge counters" - ); - if let Some(replaced) = self.bcb_counters[bcb].replace(counter_kind) { bug!( "attempt to set a BasicCoverageBlock coverage counter more than once; \ @@ -146,19 +128,6 @@ impl CoverageCounters { to_bcb: BasicCoverageBlock, counter_kind: BcbCounter, ) -> BcbCounter { - // If the BCB has an edge counter (to be injected into a new `BasicBlock`), it can also - // have an expression (to be injected into an existing `BasicBlock` represented by this - // `BasicCoverageBlock`). - if let Some(node_counter) = self.bcb_counter(to_bcb) - && !node_counter.is_expression() - { - bug!( - "attempt to add an incoming edge counter from {from_bcb:?} \ - when the target BCB already has {node_counter:?}" - ); - } - - self.bcb_has_incoming_edge_counters.insert(to_bcb); if let Some(replaced) = self.bcb_edge_counters.insert((from_bcb, to_bcb), counter_kind) { bug!( "attempt to set an edge counter more than once; from_bcb: \ diff --git a/tests/coverage/let_else_loop.coverage b/tests/coverage/let_else_loop.coverage index 3b3c39ff30180..d193c8ca1b514 100644 --- a/tests/coverage/let_else_loop.coverage +++ b/tests/coverage/let_else_loop.coverage @@ -1,6 +1,6 @@ LL| |#![feature(coverage_attribute)] LL| |//@ edition: 2021 - LL| |//@ ignore-test + LL| | LL| |// Regression test for . LL| |// These code patterns should not trigger an ICE when allocating a physical LL| |// counter to a node and also one of its in-edges, because that is allowed diff --git a/tests/coverage/let_else_loop.rs b/tests/coverage/let_else_loop.rs index 92a252cbbdc99..12e0aeabcab95 100644 --- a/tests/coverage/let_else_loop.rs +++ b/tests/coverage/let_else_loop.rs @@ -1,6 +1,6 @@ #![feature(coverage_attribute)] //@ edition: 2021 -//@ ignore-test + // Regression test for . // These code patterns should not trigger an ICE when allocating a physical // counter to a node and also one of its in-edges, because that is allowed From 2f21e4f8bb86e1877d20bf74289272ecbe2bf103 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 20 Mar 2024 18:25:06 +1100 Subject: [PATCH 10/12] coverage: Tidy imports in `rustc_mir_transform::coverage::counters` --- compiler/rustc_mir_transform/src/coverage/counters.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index 402ce6ba8d9a9..69dc4f2ddea71 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -1,12 +1,12 @@ +use std::fmt::{self, Debug}; + use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::graph::WithNumNodes; use rustc_index::IndexVec; -use rustc_middle::mir::coverage::*; +use rustc_middle::mir::coverage::{CounterId, CovTerm, Expression, ExpressionId, Op}; -use super::graph::{BasicCoverageBlock, CoverageGraph, TraverseCoverageGraphWithLoops}; - -use std::fmt::{self, Debug}; +use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph, TraverseCoverageGraphWithLoops}; /// The coverage counter or counter expression associated with a particular /// BCB node or BCB edge. From 92f668c20b155736ef8278cb02456340097f0840 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Wed, 20 Mar 2024 01:17:18 -0700 Subject: [PATCH 11/12] Add usize::MAX arg tests for Vec --- library/alloc/tests/vec.rs | 41 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index aa95b4e977081..f1f841fe190f0 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -2643,3 +2643,44 @@ fn test_vec_from_array_ref() { fn test_vec_from_array_mut_ref() { assert_eq!(Vec::from(&mut [1, 2, 3]), vec![1, 2, 3]); } + +/// This assortment of tests, in combination with miri, verifies we handle UB on fishy arguments +/// in the stdlib. Draining and extending the allocation are fairly well-tested earlier, but +/// `vec.insert(usize::MAX, val)` once slipped by! +/// +/// All code that manipulates the collection types should be tested with "trivially wrong" args. +#[test] +fn max_dont_panic() { + let mut v = vec![0]; + let _ = v.get(usize::MAX); + v.shrink_to(usize::MAX); + v.truncate(usize::MAX); +} + +#[test] +#[should_panic] +fn max_insert() { + let mut v = vec![0]; + v.insert(usize::MAX, 1); +} + +#[test] +#[should_panic] +fn max_remove() { + let mut v = vec![0]; + v.remove(usize::MAX); +} + +#[test] +#[should_panic] +fn max_splice() { + let mut v = vec![0]; + v.splice(usize::MAX.., core::iter::once(1)); +} + +#[test] +#[should_panic] +fn max_swap_remove() { + let mut v = vec![0]; + v.swap_remove(usize::MAX); +} From 98e66553a607ad9cfc492a6def69b404507becf4 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 20 Mar 2024 16:47:11 +0100 Subject: [PATCH 12/12] Rename `hir::Let` into `hir::LetExpr` --- compiler/rustc_ast_lowering/src/expr.rs | 2 +- compiler/rustc_hir/src/hir.rs | 4 ++-- compiler/rustc_hir/src/intravisit.rs | 2 +- compiler/rustc_hir_pretty/src/lib.rs | 2 +- compiler/rustc_hir_typeck/src/_match.rs | 2 +- compiler/rustc_hir_typeck/src/expr.rs | 6 +++++- compiler/rustc_hir_typeck/src/expr_use_visitor.rs | 2 +- compiler/rustc_hir_typeck/src/gather_locals.rs | 8 ++++---- .../clippy/clippy_lints/src/pattern_type_mismatch.rs | 4 ++-- src/tools/clippy/clippy_lints/src/shadow.rs | 4 ++-- src/tools/clippy/clippy_lints/src/unused_io_amount.rs | 2 +- src/tools/clippy/clippy_utils/src/higher.rs | 4 ++-- src/tools/clippy/clippy_utils/src/hir_utils.rs | 4 ++-- src/tools/clippy/clippy_utils/src/visitors.rs | 4 ++-- 14 files changed, 27 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index 41f7418ddde42..d802dbbcb9e67 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -157,7 +157,7 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::ExprKind::AddrOf(*k, *m, ohs) } ExprKind::Let(pat, scrutinee, span, is_recovered) => { - hir::ExprKind::Let(self.arena.alloc(hir::Let { + hir::ExprKind::Let(self.arena.alloc(hir::LetExpr { span: self.lower_span(*span), pat: self.lower_pat(pat), ty: None, diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index ca5d2930e82c4..d0446785e4ed2 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1259,7 +1259,7 @@ pub struct Arm<'hir> { /// In an `if let`, imagine it as `if (let = ) { ... }`; in a let-else, it is part of /// the desugaring to if-let. Only let-else supports the type annotation at present. #[derive(Debug, Clone, Copy, HashStable_Generic)] -pub struct Let<'hir> { +pub struct LetExpr<'hir> { pub span: Span, pub pat: &'hir Pat<'hir>, pub ty: Option<&'hir Ty<'hir>>, @@ -1852,7 +1852,7 @@ pub enum ExprKind<'hir> { /// /// These are not `Local` and only occur as expressions. /// The `let Some(x) = foo()` in `if let Some(x) = foo()` is an example of `Let(..)`. - Let(&'hir Let<'hir>), + Let(&'hir LetExpr<'hir>), /// An `if` block, with an optional else block. /// /// I.e., `if { } else { }`. diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index 186bb234a450b..3cf1093eeec72 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -753,7 +753,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>) ExprKind::DropTemps(ref subexpression) => { try_visit!(visitor.visit_expr(subexpression)); } - ExprKind::Let(Let { span: _, pat, ty, init, is_recovered: _ }) => { + ExprKind::Let(LetExpr { span: _, pat, ty, init, is_recovered: _ }) => { // match the visit order in walk_local try_visit!(visitor.visit_expr(init)); try_visit!(visitor.visit_pat(pat)); diff --git a/compiler/rustc_hir_pretty/src/lib.rs b/compiler/rustc_hir_pretty/src/lib.rs index 34c245839478f..c7651cf3264a5 100644 --- a/compiler/rustc_hir_pretty/src/lib.rs +++ b/compiler/rustc_hir_pretty/src/lib.rs @@ -1387,7 +1387,7 @@ impl<'a> State<'a> { // Print `}`: self.bclose_maybe_open(expr.span, true); } - hir::ExprKind::Let(&hir::Let { pat, ty, init, .. }) => { + hir::ExprKind::Let(&hir::LetExpr { pat, ty, init, .. }) => { self.print_let(pat, ty, init); } hir::ExprKind::If(test, blk, elseopt) => { diff --git a/compiler/rustc_hir_typeck/src/_match.rs b/compiler/rustc_hir_typeck/src/_match.rs index 4b3359858f15d..b6421a10e133d 100644 --- a/compiler/rustc_hir_typeck/src/_match.rs +++ b/compiler/rustc_hir_typeck/src/_match.rs @@ -317,7 +317,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { err.note("`if` expressions without `else` evaluate to `()`"); err.help("consider adding an `else` block that evaluates to the expected type"); *error = true; - if let ExprKind::Let(hir::Let { span, pat, init, .. }) = cond_expr.kind + if let ExprKind::Let(hir::LetExpr { span, pat, init, .. }) = cond_expr.kind && let ExprKind::Block(block, _) = then_expr.kind // Refutability checks occur on the MIR, so we approximate it here by checking // if we have an enum with a single variant or a struct in the pattern. diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 1a142f27809e1..4c01c201ad238 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -1261,7 +1261,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } - pub(super) fn check_expr_let(&self, let_expr: &'tcx hir::Let<'tcx>, hir_id: HirId) -> Ty<'tcx> { + pub(super) fn check_expr_let( + &self, + let_expr: &'tcx hir::LetExpr<'tcx>, + hir_id: HirId, + ) -> Ty<'tcx> { // for let statements, this is done in check_stmt let init = let_expr.init; self.warn_if_unreachable(init.hir_id, init.span, "block in `let` expression"); diff --git a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs index 43e9554459439..2dc355b72f61a 100644 --- a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs @@ -245,7 +245,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { } } - hir::ExprKind::Let(hir::Let { pat, init, .. }) => { + hir::ExprKind::Let(hir::LetExpr { pat, init, .. }) => { self.walk_local(init, pat, None, |t| t.borrow_expr(init, ty::ImmBorrow)) } diff --git a/compiler/rustc_hir_typeck/src/gather_locals.rs b/compiler/rustc_hir_typeck/src/gather_locals.rs index 4d37f725c9400..6dae1fab6dcde 100644 --- a/compiler/rustc_hir_typeck/src/gather_locals.rs +++ b/compiler/rustc_hir_typeck/src/gather_locals.rs @@ -29,7 +29,7 @@ impl<'a> DeclOrigin<'a> { } } -/// A declaration is an abstraction of [hir::Local] and [hir::Let]. +/// A declaration is an abstraction of [hir::Local] and [hir::LetExpr]. /// /// It must have a hir_id, as this is how we connect gather_locals to the check functions. pub(super) struct Declaration<'a> { @@ -48,9 +48,9 @@ impl<'a> From<&'a hir::Local<'a>> for Declaration<'a> { } } -impl<'a> From<(&'a hir::Let<'a>, hir::HirId)> for Declaration<'a> { - fn from((let_expr, hir_id): (&'a hir::Let<'a>, hir::HirId)) -> Self { - let hir::Let { pat, ty, span, init, is_recovered: _ } = *let_expr; +impl<'a> From<(&'a hir::LetExpr<'a>, hir::HirId)> for Declaration<'a> { + fn from((let_expr, hir_id): (&'a hir::LetExpr<'a>, hir::HirId)) -> Self { + let hir::LetExpr { pat, ty, span, init, is_recovered: _ } = *let_expr; Declaration { hir_id, pat, ty, span, init: Some(init), origin: DeclOrigin::LetExpr } } } diff --git a/src/tools/clippy/clippy_lints/src/pattern_type_mismatch.rs b/src/tools/clippy/clippy_lints/src/pattern_type_mismatch.rs index fbca4329342a9..127801de7defa 100644 --- a/src/tools/clippy/clippy_lints/src/pattern_type_mismatch.rs +++ b/src/tools/clippy/clippy_lints/src/pattern_type_mismatch.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_help; -use rustc_hir::{intravisit, Body, Expr, ExprKind, FnDecl, Let, LocalSource, Mutability, Pat, PatKind, Stmt, StmtKind}; +use rustc_hir::{intravisit, Body, Expr, ExprKind, FnDecl, LetExpr, LocalSource, Mutability, Pat, PatKind, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_middle::ty; @@ -103,7 +103,7 @@ impl<'tcx> LateLintPass<'tcx> for PatternTypeMismatch { } } } - if let ExprKind::Let(Let { pat, .. }) = expr.kind { + if let ExprKind::Let(LetExpr { pat, .. }) = expr.kind { apply_lint(cx, pat, DerefPossible::Possible); } } diff --git a/src/tools/clippy/clippy_lints/src/shadow.rs b/src/tools/clippy/clippy_lints/src/shadow.rs index c74364d89d61b..df7bd4c8d1d39 100644 --- a/src/tools/clippy/clippy_lints/src/shadow.rs +++ b/src/tools/clippy/clippy_lints/src/shadow.rs @@ -5,7 +5,7 @@ use rustc_data_structures::fx::FxHashMap; use rustc_hir::def::Res; use rustc_hir::def_id::LocalDefId; use rustc_hir::hir_id::ItemLocalId; -use rustc_hir::{Block, Body, BodyOwnerKind, Expr, ExprKind, HirId, Let, Node, Pat, PatKind, QPath, UnOp}; +use rustc_hir::{Block, Body, BodyOwnerKind, Expr, ExprKind, HirId, LetExpr, Node, Pat, PatKind, QPath, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::impl_lint_pass; use rustc_span::{Span, Symbol}; @@ -238,7 +238,7 @@ fn find_init<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Option<&'tcx Expr<' let init = match node { Node::Arm(_) | Node::Pat(_) => continue, Node::Expr(expr) => match expr.kind { - ExprKind::Match(e, _, _) | ExprKind::Let(&Let { init: e, .. }) => Some(e), + ExprKind::Match(e, _, _) | ExprKind::Let(&LetExpr { init: e, .. }) => Some(e), _ => None, }, Node::Local(local) => local.init, diff --git a/src/tools/clippy/clippy_lints/src/unused_io_amount.rs b/src/tools/clippy/clippy_lints/src/unused_io_amount.rs index eb64dd633f606..1497d883dfc48 100644 --- a/src/tools/clippy/clippy_lints/src/unused_io_amount.rs +++ b/src/tools/clippy/clippy_lints/src/unused_io_amount.rs @@ -131,7 +131,7 @@ fn non_consuming_ok_arm<'a>(cx: &LateContext<'a>, arm: &hir::Arm<'a>) -> bool { fn check_expr<'a>(cx: &LateContext<'a>, expr: &'a hir::Expr<'a>) { match expr.kind { hir::ExprKind::If(cond, _, _) - if let ExprKind::Let(hir::Let { pat, init, .. }) = cond.kind + if let ExprKind::Let(hir::LetExpr { pat, init, .. }) = cond.kind && is_ok_wild_or_dotdot_pattern(cx, pat) && let Some(op) = should_lint(cx, init) => { diff --git a/src/tools/clippy/clippy_utils/src/higher.rs b/src/tools/clippy/clippy_utils/src/higher.rs index ba682813dadf8..8ce19998a0828 100644 --- a/src/tools/clippy/clippy_utils/src/higher.rs +++ b/src/tools/clippy/clippy_utils/src/higher.rs @@ -102,7 +102,7 @@ impl<'hir> IfLet<'hir> { if let ExprKind::If( Expr { kind: - ExprKind::Let(&hir::Let { + ExprKind::Let(&hir::LetExpr { pat: let_pat, init: let_expr, span: let_span, @@ -379,7 +379,7 @@ impl<'hir> WhileLet<'hir> { ExprKind::If( Expr { kind: - ExprKind::Let(&hir::Let { + ExprKind::Let(&hir::LetExpr { pat: let_pat, init: let_expr, span: let_span, diff --git a/src/tools/clippy/clippy_utils/src/hir_utils.rs b/src/tools/clippy/clippy_utils/src/hir_utils.rs index 106d1d0d77f01..162bf24d85d29 100644 --- a/src/tools/clippy/clippy_utils/src/hir_utils.rs +++ b/src/tools/clippy/clippy_utils/src/hir_utils.rs @@ -8,7 +8,7 @@ use rustc_hir::def::Res; use rustc_hir::MatchSource::TryDesugar; use rustc_hir::{ ArrayLen, BinOpKind, BindingAnnotation, Block, BodyId, Closure, Expr, ExprField, ExprKind, FnRetTy, GenericArg, - GenericArgs, HirId, HirIdMap, InlineAsmOperand, Let, Lifetime, LifetimeName, Pat, PatField, PatKind, Path, + GenericArgs, HirId, HirIdMap, InlineAsmOperand, LetExpr, Lifetime, LifetimeName, Pat, PatField, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, Ty, TyKind, TypeBinding, }; use rustc_lexer::{tokenize, TokenKind}; @@ -837,7 +837,7 @@ impl<'a, 'tcx> SpanlessHash<'a, 'tcx> { } } }, - ExprKind::Let(Let { pat, init, ty, .. }) => { + ExprKind::Let(LetExpr { pat, init, ty, .. }) => { self.hash_expr(init); if let Some(ty) = ty { self.hash_ty(ty); diff --git a/src/tools/clippy/clippy_utils/src/visitors.rs b/src/tools/clippy/clippy_utils/src/visitors.rs index ebc38e531fe6e..0a05ac029eae5 100644 --- a/src/tools/clippy/clippy_utils/src/visitors.rs +++ b/src/tools/clippy/clippy_utils/src/visitors.rs @@ -5,7 +5,7 @@ use rustc_hir as hir; use rustc_hir::def::{CtorKind, DefKind, Res}; use rustc_hir::intravisit::{self, walk_block, walk_expr, Visitor}; use rustc_hir::{ - AnonConst, Arm, Block, BlockCheckMode, Body, BodyId, Expr, ExprKind, HirId, ItemId, ItemKind, Let, Pat, QPath, + AnonConst, Arm, Block, BlockCheckMode, Body, BodyId, Expr, ExprKind, HirId, ItemId, ItemKind, LetExpr, Pat, QPath, Stmt, UnOp, UnsafeSource, Unsafety, }; use rustc_lint::LateContext; @@ -624,7 +624,7 @@ pub fn for_each_unconsumed_temporary<'tcx, B>( | ExprKind::Field(e, _) | ExprKind::Unary(UnOp::Deref, e) | ExprKind::Match(e, ..) - | ExprKind::Let(&Let { init: e, .. }) => { + | ExprKind::Let(&LetExpr { init: e, .. }) => { helper(typeck, false, e, f)?; }, ExprKind::Block(&Block { expr: Some(e), .. }, _) | ExprKind::Cast(e, _) | ExprKind::Unary(_, e) => {