From c780e55995dd29ac3e6577af8e8e1daa28998a63 Mon Sep 17 00:00:00 2001 From: Scott McMurray Date: Sun, 4 Jun 2023 11:09:20 -0700 Subject: [PATCH] Dedup some type checks in the MIR validator --- .../rustc_const_eval/src/interpret/step.rs | 30 +------- .../src/transform/validate.rs | 77 +++++++------------ compiler/rustc_const_eval/src/util/mod.rs | 26 +++++++ 3 files changed, 57 insertions(+), 76 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/step.rs b/compiler/rustc_const_eval/src/interpret/step.rs index 050ed291b4f..619da8abb7d 100644 --- a/compiler/rustc_const_eval/src/interpret/step.rs +++ b/compiler/rustc_const_eval/src/interpret/step.rs @@ -9,29 +9,7 @@ use rustc_middle::mir::interpret::{InterpResult, Scalar}; use rustc_middle::ty::layout::LayoutOf; use super::{ImmTy, InterpCx, Machine}; - -/// Classify whether an operator is "left-homogeneous", i.e., the LHS has the -/// same type as the result. -#[inline] -fn binop_left_homogeneous(op: mir::BinOp) -> bool { - use rustc_middle::mir::BinOp::*; - match op { - Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor - | BitAnd | BitOr | Offset | Shl | ShlUnchecked | Shr | ShrUnchecked => true, - Eq | Ne | Lt | Le | Gt | Ge => false, - } -} -/// Classify whether an operator is "right-homogeneous", i.e., the RHS has the -/// same type as the LHS. -#[inline] -fn binop_right_homogeneous(op: mir::BinOp) -> bool { - use rustc_middle::mir::BinOp::*; - match op { - Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor - | BitAnd | BitOr | Eq | Ne | Lt | Le | Gt | Ge => true, - Offset | Shl | ShlUnchecked | Shr | ShrUnchecked => false, - } -} +use crate::util; impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Returns `true` as long as there are more things to do. @@ -181,9 +159,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } BinaryOp(bin_op, box (ref left, ref right)) => { - let layout = binop_left_homogeneous(bin_op).then_some(dest.layout); + let layout = util::binop_left_homogeneous(bin_op).then_some(dest.layout); let left = self.read_immediate(&self.eval_operand(left, layout)?)?; - let layout = binop_right_homogeneous(bin_op).then_some(left.layout); + let layout = util::binop_right_homogeneous(bin_op).then_some(left.layout); let right = self.read_immediate(&self.eval_operand(right, layout)?)?; self.binop_ignore_overflow(bin_op, &left, &right, &dest)?; } @@ -191,7 +169,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { CheckedBinaryOp(bin_op, box (ref left, ref right)) => { // Due to the extra boolean in the result, we can never reuse the `dest.layout`. let left = self.read_immediate(&self.eval_operand(left, None)?)?; - let layout = binop_right_homogeneous(bin_op).then_some(left.layout); + let layout = util::binop_right_homogeneous(bin_op).then_some(left.layout); let right = self.read_immediate(&self.eval_operand(right, layout)?)?; self.binop_with_overflow(bin_op, &left, &right, &dest)?; } diff --git a/compiler/rustc_const_eval/src/transform/validate.rs b/compiler/rustc_const_eval/src/transform/validate.rs index d421c7a2755..f197541da5b 100644 --- a/compiler/rustc_const_eval/src/transform/validate.rs +++ b/compiler/rustc_const_eval/src/transform/validate.rs @@ -498,8 +498,8 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) { macro_rules! check_kinds { - ($t:expr, $text:literal, $($patterns:tt)*) => { - if !matches!(($t).kind(), $($patterns)*) { + ($t:expr, $text:literal, $typat:pat) => { + if !matches!(($t).kind(), $typat) { self.fail(location, format!($text, $t)); } }; @@ -527,6 +527,25 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { use BinOp::*; let a = vals.0.ty(&self.body.local_decls, self.tcx); let b = vals.1.ty(&self.body.local_decls, self.tcx); + if crate::util::binop_right_homogeneous(*op) { + if let Eq | Lt | Le | Ne | Ge | Gt = op { + // The function pointer types can have lifetimes + if !self.mir_assign_valid_types(a, b) { + self.fail( + location, + format!("Cannot {op:?} compare incompatible types {a:?} and {b:?}"), + ); + } + } else if a != b { + self.fail( + location, + format!( + "Cannot perform binary op {op:?} on unequal types {a:?} and {b:?}" + ), + ); + } + } + match op { Offset => { check_kinds!(a, "Cannot offset non-pointer type {:?}", ty::RawPtr(..)); @@ -538,7 +557,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { for x in [a, b] { check_kinds!( x, - "Cannot compare type {:?}", + "Cannot {op:?} compare type {:?}", ty::Bool | ty::Char | ty::Int(..) @@ -548,19 +567,13 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { | ty::FnPtr(..) ) } - // The function pointer types can have lifetimes - if !self.mir_assign_valid_types(a, b) { - self.fail( - location, - format!("Cannot compare unequal types {:?} and {:?}", a, b), - ); - } } - Shl | ShlUnchecked | Shr | ShrUnchecked => { + AddUnchecked | SubUnchecked | MulUnchecked | Shl | ShlUnchecked | Shr + | ShrUnchecked => { for x in [a, b] { check_kinds!( x, - "Cannot shift non-integer type {:?}", + "Cannot {op:?} non-integer type {:?}", ty::Uint(..) | ty::Int(..) ) } @@ -569,55 +582,19 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> { for x in [a, b] { check_kinds!( x, - "Cannot perform bitwise op on type {:?}", + "Cannot perform bitwise op {op:?} on type {:?}", ty::Uint(..) | ty::Int(..) | ty::Bool ) } - if a != b { - self.fail( - location, - format!( - "Cannot perform bitwise op on unequal types {:?} and {:?}", - a, b - ), - ); - } } Add | Sub | Mul | Div | Rem => { for x in [a, b] { check_kinds!( x, - "Cannot perform arithmetic on type {:?}", + "Cannot perform arithmetic {op:?} on type {:?}", ty::Uint(..) | ty::Int(..) | ty::Float(..) ) } - if a != b { - self.fail( - location, - format!( - "Cannot perform arithmetic on unequal types {:?} and {:?}", - a, b - ), - ); - } - } - AddUnchecked | SubUnchecked | MulUnchecked => { - for x in [a, b] { - check_kinds!( - x, - "Cannot perform unchecked arithmetic on type {:?}", - ty::Uint(..) | ty::Int(..) - ) - } - if a != b { - self.fail( - location, - format!( - "Cannot perform unchecked arithmetic on unequal types {:?} and {:?}", - a, b - ), - ); - } } } } diff --git a/compiler/rustc_const_eval/src/util/mod.rs b/compiler/rustc_const_eval/src/util/mod.rs index 7641f560714..289e3422595 100644 --- a/compiler/rustc_const_eval/src/util/mod.rs +++ b/compiler/rustc_const_eval/src/util/mod.rs @@ -1,3 +1,5 @@ +use rustc_middle::mir; + mod alignment; mod check_validity_requirement; mod compare_types; @@ -7,3 +9,27 @@ pub use self::alignment::is_disaligned; pub use self::check_validity_requirement::check_validity_requirement; pub use self::compare_types::{is_equal_up_to_subtyping, is_subtype}; pub use self::type_name::type_name; + +/// Classify whether an operator is "left-homogeneous", i.e., the LHS has the +/// same type as the result. +#[inline] +pub(crate) fn binop_left_homogeneous(op: mir::BinOp) -> bool { + use rustc_middle::mir::BinOp::*; + match op { + Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor + | BitAnd | BitOr | Offset | Shl | ShlUnchecked | Shr | ShrUnchecked => true, + Eq | Ne | Lt | Le | Gt | Ge => false, + } +} + +/// Classify whether an operator is "right-homogeneous", i.e., the RHS has the +/// same type as the LHS. +#[inline] +pub(crate) fn binop_right_homogeneous(op: mir::BinOp) -> bool { + use rustc_middle::mir::BinOp::*; + match op { + Add | AddUnchecked | Sub | SubUnchecked | Mul | MulUnchecked | Div | Rem | BitXor + | BitAnd | BitOr | Eq | Ne | Lt | Le | Gt | Ge => true, + Offset | Shl | ShlUnchecked | Shr | ShrUnchecked => false, + } +}