Address stylistic review comments and rebase fallout

src/librustc/mir/interpret/value.rs

@@ -28,8 +28,11 @@ impl<'tcx> ConstValue<'tcx> {
     pub fn from_byval_value(val: Value) -> EvalResult<'static, Self> {
         Ok(match val {
             Value::ByRef(..) => bug!(),
-            Value::ScalarPair(a, b) => ConstValue::ScalarPair(a.read()?, b.read()?),
+            Value::ScalarPair(a, b) => ConstValue::ScalarPair(
-            Value::Scalar(val) => ConstValue::Scalar(val.read()?),
+                a.unwrap_or_err()?,
+                b.unwrap_or_err()?,
+            ),
+            Value::Scalar(val) => ConstValue::Scalar(val.unwrap_or_err()?),
         })
     }
 
@@ -197,7 +200,7 @@ impl From<Scalar> for ScalarMaybeUndef {
 }
 
 impl ScalarMaybeUndef {
-    pub fn read(self) -> EvalResult<'static, Scalar> {
+    pub fn unwrap_or_err(self) -> EvalResult<'static, Scalar> {
         match self {
             ScalarMaybeUndef::Scalar(scalar) => Ok(scalar),
             ScalarMaybeUndef::Undef => err!(ReadUndefBytes),

src/librustc_mir/interpret/const_eval.rs

@@ -88,8 +88,8 @@ pub fn value_to_const_value<'tcx>(
     }
     let val = (|| {
         match val {
-            Value::Scalar(val) => Ok(ConstValue::Scalar(val.read()?)),
+            Value::Scalar(val) => Ok(ConstValue::Scalar(val.unwrap_or_err()?)),
-            Value::ScalarPair(a, b) => Ok(ConstValue::ScalarPair(a.read()?, b.read()?)),
+            Value::ScalarPair(a, b) => Ok(ConstValue::ScalarPair(a.unwrap_or_err()?, b.unwrap_or_err()?)),
             Value::ByRef(ptr, align) => {
                 let ptr = ptr.to_ptr().unwrap();
                 let alloc = ecx.memory.get(ptr.alloc_id)?;
@@ -441,7 +441,7 @@ pub fn const_val_field<'a, 'tcx>(
         let place = ecx.allocate_place_for_value(value, layout, variant)?;
         let (place, layout) = ecx.place_field(place, field, layout)?;
         let (ptr, align) = place.to_ptr_align();
-        let mut new_value = Value::ByRef(ptr.read()?, align);
+        let mut new_value = Value::ByRef(ptr.unwrap_or_err()?, align);
         new_value = ecx.try_read_by_ref(new_value, layout.ty)?;
         use rustc_data_structures::indexed_vec::Idx;
         match (value, new_value) {
@@ -485,7 +485,7 @@ pub fn const_variant_index<'a, 'tcx>(
         },
         Value::ByRef(ptr, align) => (ptr, align),
     };
-    let place = Place::from_scalar_ptr(ptr, align);
+    let place = Place::from_scalar_ptr(ptr.into(), align);
     ecx.read_discriminant_as_variant_index(place, layout)
 }
 

src/librustc_mir/interpret/eval_context.rs

@@ -616,7 +616,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
                 if let Place::Ptr { ptr, .. } = frame.return_place {
                     // FIXME: to_ptr()? might be too extreme here, static zsts might reach this under certain conditions
                     self.memory.mark_static_initialized(
-                        ptr.read()?.to_ptr()?.alloc_id,
+                        ptr.unwrap_or_err()?.to_ptr()?.alloc_id,
                         mutable,
                     )?
                 } else {
@@ -744,7 +744,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
                 let (dest, dest_align) = self.force_allocation(dest)?.to_ptr_align();
 
                 if length > 0 {
-                    let dest = dest.read()?;
+                    let dest = dest.unwrap_or_err()?;
                     //write the first value
                     self.write_value_to_ptr(value, dest, dest_align, elem_ty)?;
 
@@ -1082,7 +1082,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
             },
         };
         Ok(Place::Ptr {
-            ptr,
+            ptr: ptr.into(),
             align,
             extra: variant.map_or(PlaceExtra::None, PlaceExtra::DowncastVariant),
         })
@@ -1120,7 +1120,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
 
     /// ensures this Value is not a ByRef
     pub fn follow_by_ref_value(
-        &mut self,
+        &self,
         value: Value,
         ty: Ty<'tcx>,
     ) -> EvalResult<'tcx, Value> {
@@ -1133,13 +1133,13 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
     }
 
     pub fn value_to_scalar(
-        &mut self,
+        &self,
         ValTy { value, ty } : ValTy<'tcx>,
     ) -> EvalResult<'tcx, Scalar> {
         match self.follow_by_ref_value(value, ty)? {
             Value::ByRef { .. } => bug!("follow_by_ref_value can't result in `ByRef`"),
 
-            Value::Scalar(scalar) => Ok(scalar),
+            Value::Scalar(scalar) => scalar.unwrap_or_err(),
 
             Value::ScalarPair(..) => bug!("value_to_scalar can't work with fat pointers"),
         }
@@ -1179,7 +1179,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
         match dest {
             Place::Ptr { ptr, align, extra } => {
                 assert_eq!(extra, PlaceExtra::None);
-                self.write_value_to_ptr(src_val, ptr.read()?, align, dest_ty)
+                self.write_value_to_ptr(src_val, ptr.unwrap_or_err()?, align, dest_ty)
             }
 
             Place::Local { frame, local } => {
@@ -1288,37 +1288,6 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
         }
     }
 
-    pub(crate) fn read_ptr(
-        &self,
-        ptr: Pointer,
-        ptr_align: Align,
-        pointee_ty: Ty<'tcx>,
-    ) -> EvalResult<'tcx, Value> {
-        let ptr_size = self.memory.pointer_size();
-        let p: ScalarMaybeUndef = self.memory.read_ptr_sized(ptr, ptr_align)?;
-        if self.type_is_sized(pointee_ty) {
-            Ok(Value::Scalar(p))
-        } else {
-            trace!("reading fat pointer extra of type {}", pointee_ty);
-            let extra = ptr.offset(ptr_size, self)?;
-            match self.tcx.struct_tail(pointee_ty).sty {
-                ty::TyDynamic(..) => Ok(Value::ScalarPair(
-                    p,
-                    self.memory.read_ptr_sized(extra, ptr_align)?,
-                )),
-                ty::TySlice(..) | ty::TyStr => {
-                    let len = self
-                        .memory
-                        .read_ptr_sized(extra, ptr_align)?
-                        .read()?
-                        .to_bits(ptr_size)?;
-                    Ok(p.to_value_with_len(len as u64, self.tcx.tcx))
-                },
-                _ => bug!("unsized scalar ptr read from {:?}", pointee_ty),
-            }
-        }
-    }
-
     fn validate_scalar(
         &self,
         value: Scalar,
@@ -1330,8 +1299,11 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
         trace!("validate scalar: {:#?}, {:#?}, {:#?}, {}", value, size, scalar, ty);
         let (lo, hi) = scalar.valid_range.clone().into_inner();
 
-        let (bits, defined) = match value {
+        let bits = match value {
-            Scalar::Bits { bits, defined } => (bits, defined),
+            Scalar::Bits { bits, size: value_size } => {
+                assert_eq!(value_size as u64, size.bytes());
+                bits
+            },
             Scalar::Ptr(_) => {
                 let ptr_size = self.memory.pointer_size();
                 let ptr_max = u128::max_value() >> (128 - ptr_size.bits());
@@ -1374,30 +1346,16 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
         }
 
         use std::ops::RangeInclusive;
-        let in_range = |bound: RangeInclusive<u128>| {
+        let in_range = |bound: RangeInclusive<u128>| bound.contains(&bits);
-            defined as u64 >= size.bits() && bound.contains(&bits)
-        };
         if lo > hi {
             if in_range(0..=hi) || in_range(lo..=u128::max_value()) {
                 Ok(())
-            } else if defined as u64 >= size.bits() {
-                validation_failure!(
-                    bits,
-                    path,
-                    format!("something in the range {:?} or {:?}", ..=hi, lo..)
-                )
             } else {
                 validation_failure!("undefined bytes", path)
             }
         } else {
             if in_range(scalar.valid_range.clone()) {
                 Ok(())
-            } else if defined as u64 >= size.bits() {
-                validation_failure!(
-                    bits,
-                    path,
-                    format!("something in the range {:?}", scalar.valid_range)
-                )
             } else {
                 validation_failure!("undefined bytes", path)
             }
@@ -1455,7 +1413,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
                     // expectation.
                     layout::Abi::Scalar(ref scalar) => {
                         let size = scalar.value.size(self);
-                        let value = self.memory.read_scalar(ptr, ptr_align, size)?;
+                        let value = self.memory.read_scalar(ptr, ptr_align, size)?.unwrap_or_err()?;
                         self.validate_scalar(value, size, scalar, &path, layout.ty)?;
                         if scalar.value == Primitive::Pointer {
                             // ignore integer pointers, we can't reason about the final hardware
@@ -1538,7 +1496,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
         }
     }
 
-    pub fn try_read_by_ref(&mut self, mut val: Value, ty: Ty<'tcx>) -> EvalResult<'tcx, Value> {
+    pub fn try_read_by_ref(&self, mut val: Value, ty: Ty<'tcx>) -> EvalResult<'tcx, Value> {
         // Convert to ByVal or ScalarPair if possible
         if let Value::ByRef(ptr, align) = val {
             if let Some(read_val) = self.try_read_value(ptr, align, ty)? {
@@ -1548,7 +1506,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
         Ok(val)
     }
 
-    pub fn try_read_value(&mut self, ptr: Scalar, ptr_align: Align, ty: Ty<'tcx>) -> EvalResult<'tcx, Option<Value>> {
+    pub fn try_read_value(&self, ptr: Scalar, ptr_align: Align, ty: Ty<'tcx>) -> EvalResult<'tcx, Option<Value>> {
         let mut layout = self.layout_of(ty)?;
         self.memory.check_align(ptr, ptr_align)?;
 
@@ -1563,9 +1521,9 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
             layout::Variants::Tagged { .. } => {
                 let variant_index = self.read_discriminant_as_variant_index(
                     Place::from_ptr(ptr, ptr_align),
-                    layout.ty,
+                    layout,
                 )?;
-                layout = layout.for_variant(&self, variant_index);
+                layout = layout.for_variant(self, variant_index);
                 trace!("variant layout: {:#?}", layout);
             },
             layout::Variants::Single { .. } => {},
@@ -1578,10 +1536,10 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
             }
             layout::Abi::ScalarPair(ref a, ref b) => {
                 let (a, b) = (&a.value, &b.value);
-                let (a_size, b_size) = (a.size(&self), b.size(&self));
+                let (a_size, b_size) = (a.size(self), b.size(self));
                 let a_ptr = ptr;
-                let b_offset = a_size.abi_align(b.align(&self));
+                let b_offset = a_size.abi_align(b.align(self));
-                let b_ptr = ptr.offset(b_offset, &self)?.into();
+                let b_ptr = ptr.offset(b_offset, self)?.into();
                 let a_val = self.memory.read_scalar(a_ptr, ptr_align, a_size)?;
                 let b_val = self.memory.read_scalar(b_ptr, ptr_align, b_size)?;
                 Ok(Some(Value::ScalarPair(a_val, b_val)))
@@ -1929,7 +1887,7 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
                 ScalarMaybeUndef::Undef,
                 ScalarMaybeUndef::Undef,
             ),
-            _ => Value::ByRef(self.alloc_ptr(ty)?.into(), layout.align),
+            _ => Value::ByRef(self.alloc_ptr(layout)?.into(), layout.align),
         })
     }
 }

src/librustc_mir/interpret/memory.rs

@@ -1011,11 +1011,11 @@ pub trait HasMemory<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'mir, 'tcx>> {
                 let vtable = mem.read_ptr_sized(
                     ref_ptr.ptr_offset(mem.pointer_size(), &mem.tcx.data_layout)?.to_ptr()?,
                     align
-                )?.read()?.to_ptr()?;
+                )?.unwrap_or_err()?.to_ptr()?;
                 Ok((ptr, vtable))
             }
 
-            Value::ScalarPair(ptr, vtable) => Ok((ptr, vtable.read()?.to_ptr()?)),
+            Value::ScalarPair(ptr, vtable) => Ok((ptr, vtable.unwrap_or_err()?.to_ptr()?)),
             _ => bug!("expected ptr and vtable, got {:?}", value),
         }
     }
@@ -1031,11 +1031,11 @@ pub trait HasMemory<'a, 'mir, 'tcx: 'a + 'mir, M: Machine<'mir, 'tcx>> {
                 let len = mem.read_ptr_sized(
                     ref_ptr.ptr_offset(mem.pointer_size(), &mem.tcx.data_layout)?.to_ptr()?,
                     align
-                )?.read()?.to_bits(mem.pointer_size())? as u64;
+                )?.unwrap_or_err()?.to_bits(mem.pointer_size())? as u64;
                 Ok((ptr, len))
             }
             Value::ScalarPair(ptr, val) => {
-                let len = val.read()?.to_bits(self.memory().pointer_size())?;
+                let len = val.unwrap_or_err()?.to_bits(self.memory().pointer_size())?;
                 Ok((ptr, len as u64))
             }
             Value::Scalar(_) => bug!("expected ptr and length, got {:?}", value),

src/librustc_mir/interpret/place.rs

@@ -197,7 +197,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
         match place {
             Place::Ptr { ptr, align, extra } => {
                 assert_eq!(extra, PlaceExtra::None);
-                Ok(Value::ByRef(ptr.read()?, align))
+                Ok(Value::ByRef(ptr.unwrap_or_err()?, align))
             }
             Place::Local { frame, local } => self.stack[frame].locals[local].access(),
         }

src/librustc_mir/interpret/terminator/drop.rs

@@ -52,7 +52,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
         let instance = match ty.sty {
             ty::TyDynamic(..) => {
                 if let Value::ScalarPair(_, vtable) = arg {
-                    self.read_drop_type_from_vtable(vtable.read()?.to_ptr()?)?
+                    self.read_drop_type_from_vtable(vtable.unwrap_or_err()?.to_ptr()?)?
                 } else {
                     bug!("expected fat ptr, got {:?}", arg);
                 }

src/librustc_mir/interpret/terminator/mod.rs

@@ -392,7 +392,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
                 let fn_ptr = self.memory.read_ptr_sized(
                     vtable.offset(ptr_size * (idx as u64 + 3), &self)?,
                     ptr_align
-                )?.read()?.to_ptr()?;
+                )?.unwrap_or_err()?.to_ptr()?;
                 let instance = self.memory.get_fn(fn_ptr)?;
                 let mut args = args.to_vec();
                 let ty = self.layout_of(args[0].ty)?.field(&self, 0)?.ty;

src/librustc_mir/interpret/traits.rs

@@ -72,7 +72,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
     ) -> EvalResult<'tcx, ty::Instance<'tcx>> {
         // we don't care about the pointee type, we just want a pointer
         let pointer_align = self.tcx.data_layout.pointer_align;
-        let drop_fn = self.memory.read_ptr_sized(vtable, pointer_align)?.read()?.to_ptr()?;
+        let drop_fn = self.memory.read_ptr_sized(vtable, pointer_align)?.unwrap_or_err()?.to_ptr()?;
         self.memory.get_fn(drop_fn)
     }
 
@@ -82,11 +82,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
     ) -> EvalResult<'tcx, (Size, Align)> {
         let pointer_size = self.memory.pointer_size();
         let pointer_align = self.tcx.data_layout.pointer_align;
-        let size = self.memory.read_ptr_sized(vtable.offset(pointer_size, self)?, pointer_align)?.read()?.to_bits(pointer_size)? as u64;
+        let size = self.memory.read_ptr_sized(vtable.offset(pointer_size, self)?, pointer_align)?.unwrap_or_err()?.to_bits(pointer_size)? as u64;
         let align = self.memory.read_ptr_sized(
             vtable.offset(pointer_size * 2, self)?,
             pointer_align
-        )?.read()?.to_bits(pointer_size)? as u64;
+        )?.unwrap_or_err()?.to_bits(pointer_size)? as u64;
         Ok((Size::from_bytes(size), Align::from_bytes(align, align).unwrap()))
     }
 }

src/librustc_mir/transform/const_prop.rs

@@ -369,7 +369,7 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> {
                     Value::Scalar(Scalar::Bits {
                         bits: n as u128,
                         size: self.tcx.data_layout.pointer_size.bytes() as u8,
-                    }),
+                    }.into()),
                     self.tcx.layout_of(self.param_env.and(self.tcx.types.usize)).ok()?,
                     span,
                 )))
@@ -391,7 +391,7 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> {
                     this.ecx.value_to_scalar(ValTy { value: val.0, ty: val.1.ty })
                 })?;
                 let val = self.use_ecx(source_info, |this| this.ecx.unary_op(op, prim, val.1))?;
-                Some((Value::Scalar(val), place_layout, span))
+                Some((Value::Scalar(val.into()), place_layout, span))
             }
             Rvalue::CheckedBinaryOp(op, ref left, ref right) |
             Rvalue::BinaryOp(op, ref left, ref right) => {