Extend the MIR validator to check many more things around rvalues.

compiler/rustc_const_eval/src/transform/validate.rs

@@ -4,14 +4,13 @@ use rustc_index::bit_set::BitSet;
 use rustc_infer::infer::TyCtxtInferExt;
 use rustc_middle::mir::interpret::Scalar;
 use rustc_middle::mir::visit::{PlaceContext, Visitor};
-use rustc_middle::mir::{traversal, Place};
 use rustc_middle::mir::{
-    AggregateKind, BasicBlock, Body, BorrowKind, Local, Location, MirPass, MirPhase, Operand,
+    traversal, AggregateKind, BasicBlock, BinOp, Body, BorrowKind, Local, Location, MirPass,
-    PlaceElem, PlaceRef, ProjectionElem, Rvalue, SourceScope, Statement, StatementKind, Terminator,
+    MirPhase, Operand, Place, PlaceElem, PlaceRef, ProjectionElem, Rvalue, SourceScope, Statement,
-    TerminatorKind, START_BLOCK,
+    StatementKind, Terminator, TerminatorKind, UnOp, START_BLOCK,
 };
 use rustc_middle::ty::fold::BottomUpFolder;
-use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt, TypeFoldable};
+use rustc_middle::ty::{self, InstanceDef, ParamEnv, Ty, TyCtxt, TypeFoldable};
 use rustc_mir_dataflow::impls::MaybeStorageLive;
 use rustc_mir_dataflow::storage::AlwaysLiveLocals;
 use rustc_mir_dataflow::{Analysis, ResultsCursor};
@@ -36,6 +35,13 @@ pub struct Validator {
 
 impl<'tcx> MirPass<'tcx> for Validator {
     fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
+        // FIXME(JakobDegen): These bodies never instantiated in codegend anyway, so it's not
+        // terribly important that they pass the validator. However, I think other passes might
+        // still see them, in which case they might be surprised. It would probably be better if we
+        // didn't put this through the MIR pipeline at all.
+        if matches!(body.source.instance, InstanceDef::Intrinsic(..) | InstanceDef::Virtual(..)) {
+            return;
+        }
         let def_id = body.source.def_id();
         let param_env = tcx.param_env(def_id);
         let mir_phase = self.mir_phase;
@@ -248,40 +254,20 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
         }
     }
 
-    fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
+    fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) {
-        match &statement.kind {
+        macro_rules! check_kinds {
-            StatementKind::Assign(box (dest, rvalue)) => {
+            ($t:expr, $text:literal, $($patterns:tt)*) => {
-                // LHS and RHS of the assignment must have the same type.
+                if !matches!(($t).kind(), $($patterns)*) {
-                let left_ty = dest.ty(&self.body.local_decls, self.tcx).ty;
+                    self.fail(location, format!($text, $t));
-                let right_ty = rvalue.ty(&self.body.local_decls, self.tcx);
+                }
-                if !self.mir_assign_valid_types(right_ty, left_ty) {
+            };
-                    self.fail(
-                        location,
-                        format!(
-                            "encountered `{:?}` with incompatible types:\n\
-                            left-hand side has type: {}\n\
-                            right-hand side has type: {}",
-                            statement.kind, left_ty, right_ty,
-                        ),
-                    );
         }
         match rvalue {
-                    // The sides of an assignment must not alias. Currently this just checks whether the places
+            Rvalue::Use(_) => {}
-                    // are identical.
-                    Rvalue::Use(Operand::Copy(src) | Operand::Move(src)) => {
-                        if dest == src {
-                            self.fail(
-                                location,
-                                "encountered `Assign` statement with overlapping memory",
-                            );
-                        }
-                    }
             Rvalue::Aggregate(agg_kind, _) => {
                 let disallowed = match **agg_kind {
                     AggregateKind::Array(..) => false,
-                            AggregateKind::Generator(..) => {
+                    AggregateKind::Generator(..) => self.mir_phase >= MirPhase::GeneratorsLowered,
-                                self.mir_phase >= MirPhase::GeneratorsLowered
-                            }
                     _ => self.mir_phase >= MirPhase::Deaggregated,
                 };
                 if disallowed {
@@ -299,8 +285,144 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                     );
                 }
             }
+            Rvalue::Len(p) => {
+                let pty = p.ty(&self.body.local_decls, self.tcx).ty;
+                check_kinds!(
+                    pty,
+                    "Cannot compute length of non-array type {:?}",
+                    ty::Array(..) | ty::Slice(..)
+                );
+            }
+            Rvalue::BinaryOp(op, vals) | Rvalue::CheckedBinaryOp(op, vals) => {
+                use BinOp::*;
+                let a = vals.0.ty(&self.body.local_decls, self.tcx);
+                let b = vals.1.ty(&self.body.local_decls, self.tcx);
+                match op {
+                    Offset => {
+                        check_kinds!(a, "Cannot offset non-pointer type {:?}", ty::RawPtr(..));
+                        if b != self.tcx.types.isize && b != self.tcx.types.usize {
+                            self.fail(location, format!("Cannot offset by non-isize type {:?}", b));
+                        }
+                    }
+                    Eq | Lt | Le | Ne | Ge | Gt => {
+                        for x in [a, b] {
+                            check_kinds!(
+                                x,
+                                "Cannot compare type {:?}",
+                                ty::Bool
+                                    | ty::Char
+                                    | ty::Int(..)
+                                    | ty::Uint(..)
+                                    | ty::Float(..)
+                                    | ty::RawPtr(..)
+                                    | ty::FnPtr(..)
+                            )
+                        }
+                        // None of the possible types have lifetimes, so we can just compare
+                        // directly
+                        if a != b {
+                            self.fail(
+                                location,
+                                format!("Cannot compare unequal types {:?} and {:?}", a, b),
+                            );
+                        }
+                    }
+                    Shl | Shr => {
+                        for x in [a, b] {
+                            check_kinds!(
+                                x,
+                                "Cannot shift non-integer type {:?}",
+                                ty::Uint(..) | ty::Int(..)
+                            )
+                        }
+                    }
+                    BitAnd | BitOr | BitXor => {
+                        for x in [a, b] {
+                            check_kinds!(
+                                x,
+                                "Cannot perform bitwise 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 op on type {:?}",
+                                ty::Uint(..) | ty::Int(..) | ty::Float(..)
+                            )
+                        }
+                        if a != b {
+                            self.fail(
+                                location,
+                                format!("Cannot perform op on unequal types {:?} and {:?}", a, b),
+                            );
+                        }
+                    }
+                }
+            }
+            Rvalue::UnaryOp(op, operand) => {
+                let a = operand.ty(&self.body.local_decls, self.tcx);
+                match op {
+                    UnOp::Neg => {
+                        check_kinds!(a, "Cannot negate type {:?}", ty::Int(..) | ty::Float(..))
+                    }
+                    UnOp::Not => {
+                        check_kinds!(
+                            a,
+                            "Cannot binary not type {:?}",
+                            ty::Int(..) | ty::Uint(..) | ty::Bool
+                        );
+                    }
+                }
+            }
+            Rvalue::ShallowInitBox(operand, _) => {
+                let a = operand.ty(&self.body.local_decls, self.tcx);
+                check_kinds!(a, "Cannot shallow init type {:?}", ty::RawPtr(..));
+            }
             _ => {}
         }
+        self.super_rvalue(rvalue, location);
+    }
+
+    fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
+        match &statement.kind {
+            StatementKind::Assign(box (dest, rvalue)) => {
+                // LHS and RHS of the assignment must have the same type.
+                let left_ty = dest.ty(&self.body.local_decls, self.tcx).ty;
+                let right_ty = rvalue.ty(&self.body.local_decls, self.tcx);
+                if !self.mir_assign_valid_types(right_ty, left_ty) {
+                    self.fail(
+                        location,
+                        format!(
+                            "encountered `{:?}` with incompatible types:\n\
+                            left-hand side has type: {}\n\
+                            right-hand side has type: {}",
+                            statement.kind, left_ty, right_ty,
+                        ),
+                    );
+                }
+                // FIXME(JakobDegen): Check this for all rvalues, not just this one.
+                if let Rvalue::Use(Operand::Copy(src) | Operand::Move(src)) = rvalue {
+                    // The sides of an assignment must not alias. Currently this just checks whether
+                    // the places are identical.
+                    if dest == src {
+                        self.fail(
+                            location,
+                            "encountered `Assign` statement with overlapping memory",
+                        );
+                    }
+                }
             }
             StatementKind::AscribeUserType(..) => {
                 if self.mir_phase >= MirPhase::DropsLowered {

src/test/mir-opt/lower_intrinsics.rs

@@ -3,7 +3,7 @@
 #![crate_type = "lib"]
 
 // EMIT_MIR lower_intrinsics.wrapping.LowerIntrinsics.diff
-pub fn wrapping<T: Copy>(a: T, b: T) {
+pub fn wrapping(a: i32, b: i32) {
     let _x = core::intrinsics::wrapping_add(a, b);
     let _y = core::intrinsics::wrapping_sub(a, b);
     let _z = core::intrinsics::wrapping_mul(a, b);

src/test/mir-opt/lower_intrinsics.wrapping.LowerIntrinsics.diff

@@ -1,23 +1,23 @@
 - // MIR for `wrapping` before LowerIntrinsics
 + // MIR for `wrapping` after LowerIntrinsics
   
-  fn wrapping(_1: T, _2: T) -> () {
+  fn wrapping(_1: i32, _2: i32) -> () {
-      debug a => _1;                       // in scope 0 at $DIR/lower_intrinsics.rs:6:26: 6:27
+      debug a => _1;                       // in scope 0 at $DIR/lower_intrinsics.rs:6:17: 6:18
-      debug b => _2;                       // in scope 0 at $DIR/lower_intrinsics.rs:6:32: 6:33
+      debug b => _2;                       // in scope 0 at $DIR/lower_intrinsics.rs:6:25: 6:26
-      let mut _0: ();                      // return place in scope 0 at $DIR/lower_intrinsics.rs:6:38: 6:38
+      let mut _0: ();                      // return place in scope 0 at $DIR/lower_intrinsics.rs:6:33: 6:33
-      let _3: T;                           // in scope 0 at $DIR/lower_intrinsics.rs:7:9: 7:11
+      let _3: i32;                         // in scope 0 at $DIR/lower_intrinsics.rs:7:9: 7:11
-      let mut _4: T;                       // in scope 0 at $DIR/lower_intrinsics.rs:7:45: 7:46
+      let mut _4: i32;                     // in scope 0 at $DIR/lower_intrinsics.rs:7:45: 7:46
-      let mut _5: T;                       // in scope 0 at $DIR/lower_intrinsics.rs:7:48: 7:49
+      let mut _5: i32;                     // in scope 0 at $DIR/lower_intrinsics.rs:7:48: 7:49
-      let mut _7: T;                       // in scope 0 at $DIR/lower_intrinsics.rs:8:45: 8:46
+      let mut _7: i32;                     // in scope 0 at $DIR/lower_intrinsics.rs:8:45: 8:46
-      let mut _8: T;                       // in scope 0 at $DIR/lower_intrinsics.rs:8:48: 8:49
+      let mut _8: i32;                     // in scope 0 at $DIR/lower_intrinsics.rs:8:48: 8:49
-      let mut _10: T;                      // in scope 0 at $DIR/lower_intrinsics.rs:9:45: 9:46
+      let mut _10: i32;                    // in scope 0 at $DIR/lower_intrinsics.rs:9:45: 9:46
-      let mut _11: T;                      // in scope 0 at $DIR/lower_intrinsics.rs:9:48: 9:49
+      let mut _11: i32;                    // in scope 0 at $DIR/lower_intrinsics.rs:9:48: 9:49
       scope 1 {
           debug _x => _3;                  // in scope 1 at $DIR/lower_intrinsics.rs:7:9: 7:11
-          let _6: T;                       // in scope 1 at $DIR/lower_intrinsics.rs:8:9: 8:11
+          let _6: i32;                     // in scope 1 at $DIR/lower_intrinsics.rs:8:9: 8:11
           scope 2 {
               debug _y => _6;              // in scope 2 at $DIR/lower_intrinsics.rs:8:9: 8:11
-              let _9: T;                   // in scope 2 at $DIR/lower_intrinsics.rs:9:9: 9:11
+              let _9: i32;                 // in scope 2 at $DIR/lower_intrinsics.rs:9:9: 9:11
               scope 3 {
                   debug _z => _9;          // in scope 3 at $DIR/lower_intrinsics.rs:9:9: 9:11
               }
@@ -30,10 +30,10 @@
           _4 = _1;                         // scope 0 at $DIR/lower_intrinsics.rs:7:45: 7:46
           StorageLive(_5);                 // scope 0 at $DIR/lower_intrinsics.rs:7:48: 7:49
           _5 = _2;                         // scope 0 at $DIR/lower_intrinsics.rs:7:48: 7:49
--         _3 = wrapping_add::<T>(move _4, move _5) -> bb1; // scope 0 at $DIR/lower_intrinsics.rs:7:14: 7:50
+-         _3 = wrapping_add::<i32>(move _4, move _5) -> bb1; // scope 0 at $DIR/lower_intrinsics.rs:7:14: 7:50
 -                                          // mir::Constant
 -                                          // + span: $DIR/lower_intrinsics.rs:7:14: 7:44
--                                          // + literal: Const { ty: extern "rust-intrinsic" fn(T, T) -> T {wrapping_add::<T>}, val: Value(Scalar(<ZST>)) }
+-                                          // + literal: Const { ty: extern "rust-intrinsic" fn(i32, i32) -> i32 {wrapping_add::<i32>}, val: Value(Scalar(<ZST>)) }
 +         _3 = Add(move _4, move _5);      // scope 0 at $DIR/lower_intrinsics.rs:7:14: 7:50
 +         goto -> bb1;                     // scope 0 at $DIR/lower_intrinsics.rs:7:14: 7:50
       }
@@ -46,10 +46,10 @@
           _7 = _1;                         // scope 1 at $DIR/lower_intrinsics.rs:8:45: 8:46
           StorageLive(_8);                 // scope 1 at $DIR/lower_intrinsics.rs:8:48: 8:49
           _8 = _2;                         // scope 1 at $DIR/lower_intrinsics.rs:8:48: 8:49
--         _6 = wrapping_sub::<T>(move _7, move _8) -> bb2; // scope 1 at $DIR/lower_intrinsics.rs:8:14: 8:50
+-         _6 = wrapping_sub::<i32>(move _7, move _8) -> bb2; // scope 1 at $DIR/lower_intrinsics.rs:8:14: 8:50
 -                                          // mir::Constant
 -                                          // + span: $DIR/lower_intrinsics.rs:8:14: 8:44
--                                          // + literal: Const { ty: extern "rust-intrinsic" fn(T, T) -> T {wrapping_sub::<T>}, val: Value(Scalar(<ZST>)) }
+-                                          // + literal: Const { ty: extern "rust-intrinsic" fn(i32, i32) -> i32 {wrapping_sub::<i32>}, val: Value(Scalar(<ZST>)) }
 +         _6 = Sub(move _7, move _8);      // scope 1 at $DIR/lower_intrinsics.rs:8:14: 8:50
 +         goto -> bb2;                     // scope 1 at $DIR/lower_intrinsics.rs:8:14: 8:50
       }
@@ -62,10 +62,10 @@
           _10 = _1;                        // scope 2 at $DIR/lower_intrinsics.rs:9:45: 9:46
           StorageLive(_11);                // scope 2 at $DIR/lower_intrinsics.rs:9:48: 9:49
           _11 = _2;                        // scope 2 at $DIR/lower_intrinsics.rs:9:48: 9:49
--         _9 = wrapping_mul::<T>(move _10, move _11) -> bb3; // scope 2 at $DIR/lower_intrinsics.rs:9:14: 9:50
+-         _9 = wrapping_mul::<i32>(move _10, move _11) -> bb3; // scope 2 at $DIR/lower_intrinsics.rs:9:14: 9:50
 -                                          // mir::Constant
 -                                          // + span: $DIR/lower_intrinsics.rs:9:14: 9:44
--                                          // + literal: Const { ty: extern "rust-intrinsic" fn(T, T) -> T {wrapping_mul::<T>}, val: Value(Scalar(<ZST>)) }
+-                                          // + literal: Const { ty: extern "rust-intrinsic" fn(i32, i32) -> i32 {wrapping_mul::<i32>}, val: Value(Scalar(<ZST>)) }
 +         _9 = Mul(move _10, move _11);    // scope 2 at $DIR/lower_intrinsics.rs:9:14: 9:50
 +         goto -> bb3;                     // scope 2 at $DIR/lower_intrinsics.rs:9:14: 9:50
       }
@@ -73,7 +73,7 @@
       bb3: {
           StorageDead(_11);                // scope 2 at $DIR/lower_intrinsics.rs:9:49: 9:50
           StorageDead(_10);                // scope 2 at $DIR/lower_intrinsics.rs:9:49: 9:50
-          _0 = const ();                   // scope 0 at $DIR/lower_intrinsics.rs:6:38: 10:2
+          _0 = const ();                   // scope 0 at $DIR/lower_intrinsics.rs:6:33: 10:2
           StorageDead(_9);                 // scope 2 at $DIR/lower_intrinsics.rs:10:1: 10:2
           StorageDead(_6);                 // scope 1 at $DIR/lower_intrinsics.rs:10:1: 10:2
           StorageDead(_3);                 // scope 0 at $DIR/lower_intrinsics.rs:10:1: 10:2