Simplify value field access

Author: oli-obk

src/librustc_mir/interpret/place.rs

@@ -120,29 +120,42 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
         variant: Option<usize>,
         field: mir::Field,
         base_ty: Ty<'tcx>,
-    ) -> EvalResult<'tcx, Option<(Value, Ty<'tcx>)>> {
+    ) -> EvalResult<'tcx, ValTy<'tcx>> {
         let mut base_layout = self.layout_of(base_ty)?;
         if let Some(variant_index) = variant {
             base_layout = base_layout.for_variant(self, variant_index);
         }
         let field_index = field.index();
         let field = base_layout.field(self, field_index)?;
         if field.size.bytes() == 0 {
-            return Ok(Some((Value::Scalar(Scalar::undef()), field.ty)))
+            return Ok(ValTy {
+                value: Value::Scalar(Scalar::undef()),
+                ty: field.ty,
+            });
         }
         let offset = base_layout.fields.offset(field_index);
-        match base {
+        let value = match base {
             // the field covers the entire type
             Value::ScalarPair(..) |
-            Value::Scalar(_) if offset.bytes() == 0 && field.size == base_layout.size => Ok(Some((base, field.ty))),
-            // split fat pointers, 2 element tuples, ...
+            Value::Scalar(_) if offset.bytes() == 0 && field.size == base_layout.size => base,
+            // extract fields from types with `ScalarPair` ABI
             Value::ScalarPair(a, b) => {
                 let val = if offset.bytes() == 0 { a } else { b };
-                Ok(Some((Value::Scalar(val), field.ty)))
+                Value::Scalar(val)
             },
-            // FIXME(oli-obk): figure out whether we should be calling `try_read_value` here
-            _ => Ok(None),
-        }
+            Value::ByRef(base_ptr, align) => {
+                let offset = base_layout.fields.offset(field_index);
+                let ptr = base_ptr.ptr_offset(offset, self)?;
+                let align = align.min(base_layout.align).min(field.align);
+                assert!(!field.is_unsized());
+                Value::ByRef(ptr, align)
+            },
+            Value::Scalar(val) => bug!("field access on non aggregate {:?}, {:?}", val, base_ty),
+        };
+        Ok(ValTy {
+            value,
+            ty: field.ty,
+        })
     }
 
     fn try_read_place_projection(
@@ -156,7 +169,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
         };
         let base_ty = self.place_ty(&proj.base);
         match proj.elem {
-            Field(field, _) => Ok(self.read_field(base, None, field, base_ty)?.map(|(f, _)| f)),
+            Field(field, _) => Ok(Some(self.read_field(base, None, field, base_ty)?.value)),
             // The NullablePointer cases should work fine, need to take care for normal enums
             Downcast(..) |
             Subslice { .. } |

src/librustc_mir/interpret/terminator/mod.rs

@@ -4,7 +4,7 @@ use rustc::ty::layout::LayoutOf;
 use syntax::codemap::Span;
 use rustc_target::spec::abi::Abi;
 
-use rustc::mir::interpret::{EvalResult, Value};
+use rustc::mir::interpret::EvalResult;
 use super::{EvalContext, Place, Machine, ValTy};
 
 use rustc_data_structures::indexed_vec::Idx;
@@ -345,31 +345,9 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
                             }
                             if self.frame().mir.args_iter().count() == layout.fields.count() + 1 {
                                 for (i, arg_local) in arg_locals.enumerate() {
-                                    let field = layout.field(&self, i)?;
-                                    if field.is_zst() {
-                                        continue;
-                                    }
-                                    let offset = layout.fields.offset(i);
-                                    let value = match args[1].value {
-                                        Value::ByRef(ptr, align) => Value::ByRef(
-                                            ptr.ptr_offset(offset, &self)?,
-                                            align.min(field.align),
-                                        ),
-                                        other if field.size == layout.size => {
-                                            // this is the case where the field covers the entire type
-                                            assert_eq!(offset.bytes(), 0);
-                                            other
-                                        },
-                                        Value::ScalarPair(a, _) if offset.bytes() == 0 => Value::Scalar(a),
-                                        Value::ScalarPair(_, b) => Value::Scalar(b),
-                                        Value::Scalar(_) => bug!("Scalar does not cover entire type"),
-                                    };
-                                    let dest =
-                                        self.eval_place(&mir::Place::Local(arg_local))?;
-                                    let valty = ValTy {
-                                        value,
-                                        ty: field.ty,
-                                    };
+                                    let field = mir::Field::new(i);
+                                    let valty = self.read_field(args[1].value, None, field, args[1].ty)?;
+                                    let dest = self.eval_place(&mir::Place::Local(arg_local))?;
                                     self.write_value(valty, dest)?;
                                 }
                             } else {

src/librustc_mir/transform/const_prop.rs

@@ -214,10 +214,10 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> {
                 ProjectionElem::Field(field, _) => {
                     trace!("field proj on {:?}", proj.base);
                     let (base, ty, span) = self.eval_place(&proj.base)?;
-                    let (value, field_ty) = self.use_ecx(span, |this| {
+                    let valty = self.use_ecx(span, |this| {
                         this.ecx.read_field(base, None, field, ty)
-                    })??;
-                    Some((value, field_ty, span))
+                    })?;
+                    Some((valty.value, valty.ty, span))
                 },
                 _ => None,
             },