remove an ineffective check in const_prop

Author: RalfJung

compiler/rustc_const_eval/src/interpret/place.rs

@@ -642,7 +642,7 @@ where
         // avoid force_allocation.
         let src = match self.read_immediate_raw(src)? {
             Ok(src_val) => {
-                assert!(!src.layout.is_unsized(), "cannot have unsized immediates");
+                assert!(!src.layout.is_unsized(), "cannot copy unsized immediates");
                 assert!(
                     !dest.layout.is_unsized(),
                     "the src is sized, so the dest must also be sized"

compiler/rustc_const_eval/src/interpret/projection.rs

@@ -100,6 +100,8 @@ where
         // This makes several assumptions about what layouts we will encounter; we match what
         // codegen does as good as we can (see `extract_field` in `rustc_codegen_ssa/src/mir/operand.rs`).
         let field_val: Immediate<_> = match (*base, base.layout.abi) {
+            // if the entire value is uninit, then so is the field (can happen in ConstProp)
+            (Immediate::Uninit, _) => Immediate::Uninit,
             // the field contains no information, can be left uninit
             _ if field_layout.is_zst() => Immediate::Uninit,
             // the field covers the entire type
@@ -124,6 +126,7 @@ where
                     b_val
                 })
             }
+            // everything else is a bug
             _ => span_bug!(
                 self.cur_span(),
                 "invalid field access on immediate {}, layout {:#?}",

compiler/rustc_mir_transform/src/const_prop.rs

@@ -248,16 +248,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx>
         local: Local,
     ) -> InterpResult<'tcx, &'a interpret::Operand<Self::Provenance>> {
         let l = &frame.locals[local];
-
-        if matches!(
-            l.value,
-            LocalValue::Live(interpret::Operand::Immediate(interpret::Immediate::Uninit))
-        ) {
-            // For us "uninit" means "we don't know its value, might be initiailized or not".
-            // So stop here.
-            throw_machine_stop_str!("tried to access alocal with unknown value ")
-        }
-
+        // Applying restrictions here is meaningless since they can be circumvented via `force_allocation`.
         l.access()
     }
 
@@ -431,7 +422,13 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
 
     fn get_const(&self, place: Place<'tcx>) -> Option<OpTy<'tcx>> {
         let op = match self.ecx.eval_place_to_op(place, None) {
-            Ok(op) => op,
+            Ok(op) => {
+                if matches!(*op, interpret::Operand::Immediate(Immediate::Uninit)) {
+                    // Make sure nobody accidentally uses this value.
+                    return None;
+                }
+                op
+            }
             Err(e) => {
                 trace!("get_const failed: {}", e);
                 return None;
@@ -643,6 +640,14 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
         if rvalue.needs_subst() {
             return None;
         }
+        if !rvalue
+            .ty(&self.ecx.frame().body.local_decls, *self.ecx.tcx)
+            .is_sized(self.ecx.tcx, self.param_env)
+        {
+            // the interpreter doesn't support unsized locals (only unsized arguments),
+            // but rustc does (in a kinda broken way), so we have to skip them here
+            return None;
+        }
 
         if self.tcx.sess.mir_opt_level() >= 4 {
             self.eval_rvalue_with_identities(rvalue, place)
@@ -660,18 +665,20 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
         self.use_ecx(|this| match rvalue {
             Rvalue::BinaryOp(op, box (left, right))
             | Rvalue::CheckedBinaryOp(op, box (left, right)) => {
-                let l = this.ecx.eval_operand(left, None);
-                let r = this.ecx.eval_operand(right, None);
+                let l = this.ecx.eval_operand(left, None).and_then(|x| this.ecx.read_immediate(&x));
+                let r =
+                    this.ecx.eval_operand(right, None).and_then(|x| this.ecx.read_immediate(&x));
 
                 let const_arg = match (l, r) {
-                    (Ok(ref x), Err(_)) | (Err(_), Ok(ref x)) => this.ecx.read_immediate(x)?,
-                    (Err(e), Err(_)) => return Err(e),
-                    (Ok(_), Ok(_)) => return this.ecx.eval_rvalue_into_place(rvalue, place),
+                    (Ok(x), Err(_)) | (Err(_), Ok(x)) => x, // exactly one side is known
+                    (Err(e), Err(_)) => return Err(e),      // neither side is known
+                    (Ok(_), Ok(_)) => return this.ecx.eval_rvalue_into_place(rvalue, place), // both sides are known
                 };
 
                 if !matches!(const_arg.layout.abi, abi::Abi::Scalar(..)) {
                     // We cannot handle Scalar Pair stuff.
-                    return this.ecx.eval_rvalue_into_place(rvalue, place);
+                    // No point in calling `eval_rvalue_into_place`, since only one side is known
+                    throw_machine_stop_str!("cannot optimize this")
                 }
 
                 let arg_value = const_arg.to_scalar().to_bits(const_arg.layout.size)?;
@@ -696,7 +703,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
                             this.ecx.write_immediate(*const_arg, &dest)
                         }
                     }
-                    _ => this.ecx.eval_rvalue_into_place(rvalue, place),
+                    _ => throw_machine_stop_str!("cannot optimize this"),
                 }
             }
             _ => this.ecx.eval_rvalue_into_place(rvalue, place),
@@ -1073,7 +1080,11 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
                 if let Some(ref value) = self.eval_operand(&cond) {
                     trace!("assertion on {:?} should be {:?}", value, expected);
                     let expected = Scalar::from_bool(*expected);
-                    let value_const = self.ecx.read_scalar(&value).unwrap();
+                    let Ok(value_const) = self.ecx.read_scalar(&value) else {
+                        // FIXME should be used use_ecx rather than a local match... but we have
+                        // quite a few of these read_scalar/read_immediate that need fixing.
+                        return
+                    };
                     if expected != value_const {
                         // Poison all places this operand references so that further code
                         // doesn't use the invalid value

compiler/rustc_mir_transform/src/const_prop_lint.rs

@@ -6,6 +6,7 @@ use crate::const_prop::ConstPropMachine;
 use crate::const_prop::ConstPropMode;
 use crate::MirLint;
 use rustc_const_eval::const_eval::ConstEvalErr;
+use rustc_const_eval::interpret::Immediate;
 use rustc_const_eval::interpret::{
     self, InterpCx, InterpResult, LocalState, LocalValue, MemoryKind, OpTy, Scalar, StackPopCleanup,
 };
@@ -229,7 +230,13 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
 
     fn get_const(&self, place: Place<'tcx>) -> Option<OpTy<'tcx>> {
         let op = match self.ecx.eval_place_to_op(place, None) {
-            Ok(op) => op,
+            Ok(op) => {
+                if matches!(*op, interpret::Operand::Immediate(Immediate::Uninit)) {
+                    // Make sure nobody accidentally uses this value.
+                    return None;
+                }
+                op
+            }
             Err(e) => {
                 trace!("get_const failed: {}", e);
                 return None;
@@ -515,6 +522,14 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
         if rvalue.needs_subst() {
             return None;
         }
+        if !rvalue
+            .ty(&self.ecx.frame().body.local_decls, *self.ecx.tcx)
+            .is_sized(self.ecx.tcx, self.param_env)
+        {
+            // the interpreter doesn't support unsized locals (only unsized arguments),
+            // but rustc does (in a kinda broken way), so we have to skip them here
+            return None;
+        }
 
         self.use_ecx(source_info, |this| this.ecx.eval_rvalue_into_place(rvalue, place))
     }
@@ -624,7 +639,11 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> {
                 if let Some(ref value) = self.eval_operand(&cond, source_info) {
                     trace!("assertion on {:?} should be {:?}", value, expected);
                     let expected = Scalar::from_bool(*expected);
-                    let value_const = self.ecx.read_scalar(&value).unwrap();
+                    let Ok(value_const) = self.ecx.read_scalar(&value) else {
+                        // FIXME should be used use_ecx rather than a local match... but we have
+                        // quite a few of these read_scalar/read_immediate that need fixing.
+                        return
+                    };
                     if expected != value_const {
                         enum DbgVal<T> {
                             Val(T),
@@ -641,9 +660,9 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> {
                         let mut eval_to_int = |op| {
                             // This can be `None` if the lhs wasn't const propagated and we just
                             // triggered the assert on the value of the rhs.
-                            self.eval_operand(op, source_info).map_or(DbgVal::Underscore, |op| {
-                                DbgVal::Val(self.ecx.read_immediate(&op).unwrap().to_const_int())
-                            })
+                            self.eval_operand(op, source_info)
+                                .and_then(|op| self.ecx.read_immediate(&op).ok())
+                                .map_or(DbgVal::Underscore, |op| DbgVal::Val(op.to_const_int()))
                         };
                         let msg = match msg {
                             AssertKind::DivisionByZero(op) => {

src/test/mir-opt/const_prop/mutable_variable_unprop_assign.main.ConstProp.diff

@@ -41,7 +41,8 @@
           StorageLive(_4);                 // scope 2 at $DIR/mutable_variable_unprop_assign.rs:+4:9: +4:10
           _4 = (_2.1: i32);                // scope 2 at $DIR/mutable_variable_unprop_assign.rs:+4:13: +4:16
           StorageLive(_5);                 // scope 3 at $DIR/mutable_variable_unprop_assign.rs:+5:9: +5:10
-          _5 = (_2.0: i32);                // scope 3 at $DIR/mutable_variable_unprop_assign.rs:+5:13: +5:16
+-         _5 = (_2.0: i32);                // scope 3 at $DIR/mutable_variable_unprop_assign.rs:+5:13: +5:16
++         _5 = const 1_i32;                // scope 3 at $DIR/mutable_variable_unprop_assign.rs:+5:13: +5:16
           nop;                             // scope 0 at $DIR/mutable_variable_unprop_assign.rs:+0:11: +6:2
           StorageDead(_5);                 // scope 3 at $DIR/mutable_variable_unprop_assign.rs:+6:1: +6:2
           StorageDead(_4);                 // scope 2 at $DIR/mutable_variable_unprop_assign.rs:+6:1: +6:2