without all those copies of constants, we can finally make eval_operand take &self

4 files changed, 53 insertions, 59 deletions

diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs
index 034b81852d5..1e45215238c 100644
--- a/src/librustc_mir/interpret/operand.rs
+++ b/src/librustc_mir/interpret/operand.rs
@@ -427,12 +427,12 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
     // avoid allocations.  If you already know the layout, you can pass it in
     // to avoid looking it up again.
     fn eval_place_to_op(
-        &mut self,
+        &self,
         mir_place: &mir::Place<'tcx>,
         layout: Option<TyLayout<'tcx>>,
     ) -> EvalResult<'tcx, OpTy<'tcx>> {
         use rustc::mir::Place::*;
-        Ok(match *mir_place {
+        let op = match *mir_place {
             Local(mir::RETURN_PLACE) => return err!(ReadFromReturnPointer),
             Local(local) => {
                 let op = *self.frame().locals[local].access()?;
@@ -446,21 +446,18 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
                 self.operand_projection(op, &proj.elem)?
             }
 
-            // Everything else is an mplace, so we just call `eval_place`.
-            // Note that getting an mplace for a static aways requires `&mut`,
-            // so this does not "cost" us anything in terms if mutability.
-            Promoted(_) | Static(_) => {
-                let place = self.eval_place(mir_place)?;
-                place.to_mem_place().into()
-            }
-        })
+            _ => self.eval_place_to_mplace(mir_place)?.into(),
+        };
+
+        trace!("eval_place_to_op: got {:?}", *op);
+        Ok(op)
     }
 
     /// Evaluate the operand, returning a place where you can then find the data.
     /// if you already know the layout, you can save two some table lookups
     /// by passing it in here.
     pub fn eval_operand(
-        &mut self,
+        &self,
         mir_op: &mir::Operand<'tcx>,
         layout: Option<TyLayout<'tcx>>,
     ) -> EvalResult<'tcx, OpTy<'tcx>> {
@@ -486,7 +483,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
 
     /// Evaluate a bunch of operands at once
     pub(crate) fn eval_operands(
-        &mut self,
+        &self,
         ops: &[mir::Operand<'tcx>],
     ) -> EvalResult<'tcx, Vec<OpTy<'tcx>>> {
         ops.into_iter()
@@ -495,8 +492,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
     }
 
     // Also used e.g. when miri runs into a constant.
-    // Unfortunately, this needs an `&mut` to be able to allocate a copy of a `ByRef`
-    // constant.  This bleeds up to `eval_operand` needing `&mut`.
     pub fn const_value_to_op(
         &self,
         val: ConstValue<'tcx>,
@@ -527,18 +522,6 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
         self.const_value_to_op(cv.val)
     }
 
-    /// We cannot do self.read_value(self.eval_operand) due to eval_operand taking &mut self,
-    /// so this helps avoid unnecessary let.
-    #[inline]
-    pub fn eval_operand_and_read_value(
-        &mut self,
-        op: &mir::Operand<'tcx>,
-        layout: Option<TyLayout<'tcx>>,
-    ) -> EvalResult<'tcx, ValTy<'tcx>> {
-        let op = self.eval_operand(op, layout)?;
-        self.read_value(op)
-    }
-
     /// reads a tag and produces the corresponding variant index
     pub fn read_discriminant_as_variant_index(
         &self,
diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs
index d216e503079..de458b570f0 100644
--- a/src/librustc_mir/interpret/place.rs
+++ b/src/librustc_mir/interpret/place.rs
@@ -494,33 +494,24 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
         })
     }
 
-    /// Compute a place.  You should only use this if you intend to write into this
-    /// place; for reading, a more efficient alternative is `eval_place_for_read`.
-    pub fn eval_place(&mut self, mir_place: &mir::Place<'tcx>) -> EvalResult<'tcx, PlaceTy<'tcx>> {
+    /// Evaluate statics and promoteds to an `MPlace`.  Used to share some code between
+    /// `eval_place` and `eval_place_to_op`.
+    pub(super) fn eval_place_to_mplace(
+        &self,
+        mir_place: &mir::Place<'tcx>
+    ) -> EvalResult<'tcx, MPlaceTy<'tcx>> {
         use rustc::mir::Place::*;
-        let place = match *mir_place {
-            Local(mir::RETURN_PLACE) => PlaceTy {
-                place: self.frame().return_place,
-                layout: self.layout_of_local(self.cur_frame(), mir::RETURN_PLACE)?,
-            },
-            Local(local) => PlaceTy {
-                place: Place::Local {
-                    frame: self.cur_frame(),
-                    local,
-                },
-                layout: self.layout_of_local(self.cur_frame(), local)?,
-            },
-
+        Ok(match *mir_place {
             Promoted(ref promoted) => {
                 let instance = self.frame().instance;
                 let op = self.global_to_op(GlobalId {
                     instance,
                     promoted: Some(promoted.0),
                 })?;
-                let mplace = op.to_mem_place();
+                let mplace = op.to_mem_place(); // these are always in memory
                 let ty = self.monomorphize(promoted.1, self.substs());
-                PlaceTy {
-                    place: Place::Ptr(mplace),
+                MPlaceTy {
+                    mplace,
                     layout: self.layout_of(ty)?,
                 }
             }
@@ -539,17 +530,39 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
                 // global table but not in its local memory.
                 let alloc = self.tcx.alloc_map.lock()
                     .intern_static(cid.instance.def_id());
-                MPlaceTy::from_aligned_ptr(alloc.into(), layout).into()
+                MPlaceTy::from_aligned_ptr(alloc.into(), layout)
             }
 
+            _ => bug!("eval_place_to_mplace called on {:?}", mir_place),
+        })
+    }
+
+    /// Compute a place.  You should only use this if you intend to write into this
+    /// place; for reading, a more efficient alternative is `eval_place_for_read`.
+    pub fn eval_place(&mut self, mir_place: &mir::Place<'tcx>) -> EvalResult<'tcx, PlaceTy<'tcx>> {
+        use rustc::mir::Place::*;
+        let place = match *mir_place {
+            Local(mir::RETURN_PLACE) => PlaceTy {
+                place: self.frame().return_place,
+                layout: self.layout_of_local(self.cur_frame(), mir::RETURN_PLACE)?,
+            },
+            Local(local) => PlaceTy {
+                place: Place::Local {
+                    frame: self.cur_frame(),
+                    local,
+                },
+                layout: self.layout_of_local(self.cur_frame(), local)?,
+            },
+
             Projection(ref proj) => {
                 let place = self.eval_place(&proj.base)?;
                 self.place_projection(place, &proj.elem)?
             }
+
+            _ => self.eval_place_to_mplace(mir_place)?.into(),
         };
 
         self.dump_place(place.place);
-
         Ok(place)
     }
 
diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs
index e6caca9e1c4..54fdf8e0d4b 100644
--- a/src/librustc_mir/interpret/step.rs
+++ b/src/librustc_mir/interpret/step.rs
@@ -188,9 +188,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
 
             BinaryOp(bin_op, ref left, ref right) => {
                 let layout = if binop_left_homogeneous(bin_op) { Some(dest.layout) } else { None };
-                let left = self.eval_operand_and_read_value(left, layout)?;
+                let left = self.read_value(self.eval_operand(left, layout)?)?;
                 let layout = if binop_right_homogeneous(bin_op) { Some(left.layout) } else { None };
-                let right = self.eval_operand_and_read_value(right, layout)?;
+                let right = self.read_value(self.eval_operand(right, layout)?)?;
                 self.binop_ignore_overflow(
                     bin_op,
                     left,
@@ -201,9 +201,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
 
             CheckedBinaryOp(bin_op, ref left, ref right) => {
                 // Due to the extra boolean in the result, we can never reuse the `dest.layout`.
-                let left = self.eval_operand_and_read_value(left, None)?;
+                let left = self.read_value(self.eval_operand(left, None)?)?;
                 let layout = if binop_right_homogeneous(bin_op) { Some(left.layout) } else { None };
-                let right = self.eval_operand_and_read_value(right, layout)?;
+                let right = self.read_value(self.eval_operand(right, layout)?)?;
                 self.binop_with_overflow(
                     bin_op,
                     left,
@@ -214,7 +214,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
 
             UnaryOp(un_op, ref operand) => {
                 // The operand always has the same type as the result.
-                let val = self.eval_operand_and_read_value(operand, Some(dest.layout))?;
+                let val = self.read_value(self.eval_operand(operand, Some(dest.layout))?)?;
                 let val = self.unary_op(un_op, val.to_scalar()?, dest.layout)?;
                 self.write_scalar(val, dest)?;
             }
diff --git a/src/librustc_mir/interpret/terminator/mod.rs b/src/librustc_mir/interpret/terminator/mod.rs
index 4a5699900cf..6ffee096a07 100644
--- a/src/librustc_mir/interpret/terminator/mod.rs
+++ b/src/librustc_mir/interpret/terminator/mod.rs
@@ -52,8 +52,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
                 ref targets,
                 ..
             } => {
-                let discr_val = self.eval_operand(discr, None)?;
-                let discr = self.read_value(discr_val)?;
+                let discr = self.read_value(self.eval_operand(discr, None)?)?;
                 trace!("SwitchInt({:?})", *discr);
 
                 // Branch to the `otherwise` case by default, if no match is found.
@@ -164,19 +163,18 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
                 target,
                 ..
             } => {
-                let cond_val = self.eval_operand_and_read_value(cond, None)?
-                    .to_scalar()?
-                    .to_bool()?;
+                let cond_val = self.read_value(self.eval_operand(cond, None)?)?
+                    .to_scalar()?.to_bool()?;
                 if expected == cond_val {
                     self.goto_block(Some(target))?;
                 } else {
                     use rustc::mir::interpret::EvalErrorKind::*;
                     return match *msg {
                         BoundsCheck { ref len, ref index } => {
-                            let len = self.eval_operand_and_read_value(len, None)
+                            let len = self.read_value(self.eval_operand(len, None)?)
                                 .expect("can't eval len").to_scalar()?
                                 .to_bits(self.memory().pointer_size())? as u64;
-                            let index = self.eval_operand_and_read_value(index, None)
+                            let index = self.read_value(self.eval_operand(index, None)?)
                                 .expect("can't eval index").to_scalar()?
                                 .to_bits(self.memory().pointer_size())? as u64;
                             err!(BoundsCheck { len, index })