make bin_op and unary_op APIs consistently work on ImmTy

RalfJung · RalfJung · commit b376ae6671a2 · 2019-02-13T10:14:56.000+01:00
diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs
@@ -11,7 +11,7 @@ use rustc::hir::def::Def;
 use rustc::mir::interpret::{ConstEvalErr, ErrorHandled};
 use rustc::mir;
 use rustc::ty::{self, TyCtxt, query::TyCtxtAt};
-use rustc::ty::layout::{self, LayoutOf, TyLayout, VariantIdx};
+use rustc::ty::layout::{self, LayoutOf, VariantIdx};
 use rustc::ty::subst::Subst;
 use rustc::traits::Reveal;
 use rustc_data_structures::fx::FxHashMap;
@@ -21,7 +21,8 @@ use syntax::ast::Mutability;
 use syntax::source_map::{Span, DUMMY_SP};
 
 use crate::interpret::{self,
-    PlaceTy, MPlaceTy, MemPlace, OpTy, Operand, Immediate, Scalar, RawConst, ConstValue, Pointer,
+    PlaceTy, MPlaceTy, MemPlace, OpTy, ImmTy, Operand, Immediate, Scalar, Pointer,
+    RawConst, ConstValue,
     EvalResult, EvalError, EvalErrorKind, GlobalId, EvalContext, StackPopCleanup,
     Allocation, AllocId, MemoryKind,
     snapshot, RefTracking,
@@ -379,10 +380,8 @@ impl<'a, 'mir, 'tcx> interpret::Machine<'a, 'mir, 'tcx>
     fn ptr_op(
         _ecx: &EvalContext<'a, 'mir, 'tcx, Self>,
         _bin_op: mir::BinOp,
-        _left: Scalar,
-        _left_layout: TyLayout<'tcx>,
-        _right: Scalar,
-        _right_layout: TyLayout<'tcx>,
+        _left: ImmTy<'tcx>,
+        _right: ImmTy<'tcx>,
     ) -> EvalResult<'tcx, (Scalar, bool)> {
         Err(
             ConstEvalError::NeedsRfc("pointer arithmetic or comparison".to_string()).into(),
diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs
@@ -173,7 +173,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                     "unchecked_shr" => BinOp::Shr,
                     _ => bug!("Already checked for int ops")
                 };
-                let (val, overflowed) = self.binary_op_imm(bin_op, l, r)?;
+                let (val, overflowed) = self.binary_op(bin_op, l, r)?;
                 if overflowed {
                     let layout = self.layout_of(substs.type_at(0))?;
                     let r_val =  r.to_scalar()?.to_bits(layout.size)?;
diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs
@@ -7,11 +7,11 @@ use std::hash::Hash;
 
 use rustc::hir::{self, def_id::DefId};
 use rustc::mir;
-use rustc::ty::{self, layout::TyLayout, query::TyCtxtAt};
+use rustc::ty::{self, query::TyCtxtAt};
 
 use super::{
     Allocation, AllocId, EvalResult, Scalar, AllocationExtra,
-    EvalContext, PlaceTy, MPlaceTy, OpTy, Pointer, MemoryKind,
+    EvalContext, PlaceTy, MPlaceTy, OpTy, ImmTy, Pointer, MemoryKind,
 };
 
 /// Whether this kind of memory is allowed to leak
@@ -158,10 +158,8 @@ pub trait Machine<'a, 'mir, 'tcx>: Sized {
     fn ptr_op(
         ecx: &EvalContext<'a, 'mir, 'tcx, Self>,
         bin_op: mir::BinOp,
-        left: Scalar<Self::PointerTag>,
-        left_layout: TyLayout<'tcx>,
-        right: Scalar<Self::PointerTag>,
-        right_layout: TyLayout<'tcx>,
+        left: ImmTy<'tcx, Self::PointerTag>,
+        right: ImmTy<'tcx, Self::PointerTag>,
     ) -> EvalResult<'tcx, (Scalar<Self::PointerTag>, bool)>;
 
     /// Heap allocations via the `box` keyword.
diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs
@@ -44,6 +44,11 @@ impl Immediate {
 }
 
 impl<'tcx, Tag> Immediate<Tag> {
+    #[inline]
+    pub fn from_scalar(val: Scalar<Tag>) -> Self {
+        Immediate::Scalar(ScalarMaybeUndef::Scalar(val))
+    }
+
     #[inline]
     pub fn erase_tag(self) -> Immediate
     {
@@ -115,7 +120,7 @@ impl<'tcx, Tag> Immediate<Tag> {
 // as input for binary and cast operations.
 #[derive(Copy, Clone, Debug)]
 pub struct ImmTy<'tcx, Tag=()> {
-    crate imm: Immediate<Tag>, // ideally we'd make this private, but const_prop needs this
+    pub imm: Immediate<Tag>,
     pub layout: TyLayout<'tcx>,
 }
 
@@ -215,6 +220,19 @@ impl<'tcx, Tag> From<ImmTy<'tcx, Tag>> for OpTy<'tcx, Tag> {
     }
 }
 
+impl<'tcx, Tag: Copy> ImmTy<'tcx, Tag>
+{
+    #[inline]
+    pub fn from_scalar(val: Scalar<Tag>, layout: TyLayout<'tcx>) -> Self {
+        ImmTy { imm: Immediate::from_scalar(val), layout }
+    }
+
+    #[inline]
+    pub fn to_bits(self) -> EvalResult<'tcx, u128> {
+        self.to_scalar()?.to_bits(self.layout.size)
+    }
+}
+
 impl<'tcx, Tag> OpTy<'tcx, Tag>
 {
     #[inline]
diff --git a/src/librustc_mir/interpret/operator.rs b/src/librustc_mir/interpret/operator.rs
@@ -18,7 +18,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         right: ImmTy<'tcx, M::PointerTag>,
         dest: PlaceTy<'tcx, M::PointerTag>,
     ) -> EvalResult<'tcx> {
-        let (val, overflowed) = self.binary_op_imm(op, left, right)?;
+        let (val, overflowed) = self.binary_op(op, left, right)?;
         let val = Immediate::ScalarPair(val.into(), Scalar::from_bool(overflowed).into());
         self.write_immediate(val, dest)
     }
@@ -32,7 +32,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         right: ImmTy<'tcx, M::PointerTag>,
         dest: PlaceTy<'tcx, M::PointerTag>,
     ) -> EvalResult<'tcx> {
-        let (val, _overflowed) = self.binary_op_imm(op, left, right)?;
+        let (val, _overflowed) = self.binary_op(op, left, right)?;
         self.write_scalar(val, dest)
     }
 }
@@ -272,83 +272,70 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
         Ok((val, false))
     }
 
-    /// Convenience wrapper that's useful when keeping the layout together with the
-    /// immediate value.
+    /// Returns the result of the specified operation and whether it overflowed.
     #[inline]
-    pub fn binary_op_imm(
+    pub fn binary_op(
         &self,
         bin_op: mir::BinOp,
         left: ImmTy<'tcx, M::PointerTag>,
         right: ImmTy<'tcx, M::PointerTag>,
-    ) -> EvalResult<'tcx, (Scalar<M::PointerTag>, bool)> {
-        self.binary_op(
-            bin_op,
-            left.to_scalar()?, left.layout,
-            right.to_scalar()?, right.layout,
-        )
-    }
-
-    /// Returns the result of the specified operation and whether it overflowed.
-    pub fn binary_op(
-        &self,
-        bin_op: mir::BinOp,
-        left: Scalar<M::PointerTag>,
-        left_layout: TyLayout<'tcx>,
-        right: Scalar<M::PointerTag>,
-        right_layout: TyLayout<'tcx>,
     ) -> EvalResult<'tcx, (Scalar<M::PointerTag>, bool)> {
         trace!("Running binary op {:?}: {:?} ({:?}), {:?} ({:?})",
-            bin_op, left, left_layout.ty, right, right_layout.ty);
+            bin_op, *left, left.layout.ty, *right, right.layout.ty);
 
-        match left_layout.ty.sty {
+        match left.layout.ty.sty {
             ty::Char => {
-                assert_eq!(left_layout.ty, right_layout.ty);
-                let left = left.to_char()?;
-                let right = right.to_char()?;
+                assert_eq!(left.layout.ty, right.layout.ty);
+                let left = left.to_scalar()?.to_char()?;
+                let right = right.to_scalar()?.to_char()?;
                 self.binary_char_op(bin_op, left, right)
             }
             ty::Bool => {
-                assert_eq!(left_layout.ty, right_layout.ty);
-                let left = left.to_bool()?;
-                let right = right.to_bool()?;
+                assert_eq!(left.layout.ty, right.layout.ty);
+                let left = left.to_scalar()?.to_bool()?;
+                let right = right.to_scalar()?.to_bool()?;
                 self.binary_bool_op(bin_op, left, right)
             }
             ty::Float(fty) => {
-                assert_eq!(left_layout.ty, right_layout.ty);
-                let left = left.to_bits(left_layout.size)?;
-                let right = right.to_bits(right_layout.size)?;
+                assert_eq!(left.layout.ty, right.layout.ty);
+                let left = left.to_bits()?;
+                let right = right.to_bits()?;
                 self.binary_float_op(bin_op, fty, left, right)
             }
             _ => {
                 // Must be integer(-like) types.  Don't forget about == on fn pointers.
-                assert!(left_layout.ty.is_integral() || left_layout.ty.is_unsafe_ptr() ||
-                    left_layout.ty.is_fn());
-                assert!(right_layout.ty.is_integral() || right_layout.ty.is_unsafe_ptr() ||
-                    right_layout.ty.is_fn());
+                assert!(left.layout.ty.is_integral() || left.layout.ty.is_unsafe_ptr() ||
+                    left.layout.ty.is_fn());
+                assert!(right.layout.ty.is_integral() || right.layout.ty.is_unsafe_ptr() ||
+                    right.layout.ty.is_fn());
 
                 // Handle operations that support pointer values
-                if left.is_ptr() || right.is_ptr() || bin_op == mir::BinOp::Offset {
-                    return M::ptr_op(self, bin_op, left, left_layout, right, right_layout);
+                if left.to_scalar_ptr()?.is_ptr() ||
+                    right.to_scalar_ptr()?.is_ptr() ||
+                    bin_op == mir::BinOp::Offset
+                {
+                    return M::ptr_op(self, bin_op, left, right);
                 }
 
                 // Everything else only works with "proper" bits
-                let left = left.to_bits(left_layout.size).expect("we checked is_ptr");
-                let right = right.to_bits(right_layout.size).expect("we checked is_ptr");
-                self.binary_int_op(bin_op, left, left_layout, right, right_layout)
+                let l = left.to_bits().expect("we checked is_ptr");
+                let r = right.to_bits().expect("we checked is_ptr");
+                self.binary_int_op(bin_op, l, left.layout, r, right.layout)
             }
         }
     }
 
     pub fn unary_op(
         &self,
         un_op: mir::UnOp,
-        val: Scalar<M::PointerTag>,
-        layout: TyLayout<'tcx>,
+        val: ImmTy<'tcx, M::PointerTag>,
     ) -> EvalResult<'tcx, Scalar<M::PointerTag>> {
         use rustc::mir::UnOp::*;
         use rustc_apfloat::ieee::{Single, Double};
         use rustc_apfloat::Float;
 
+        let layout = val.layout;
+        let val = val.to_scalar()?;
         trace!("Running unary op {:?}: {:?} ({:?})", un_op, val, layout.ty.sty);
 
         match layout.ty.sty {
diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs
@@ -176,7 +176,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, '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.read_immediate(self.eval_operand(operand, Some(dest.layout))?)?;
-                let val = self.unary_op(un_op, val.to_scalar()?, dest.layout)?;
+                let val = self.unary_op(un_op, val)?;
                 self.write_scalar(val, dest)?;
             }
 
diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs
@@ -51,8 +51,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>
                     // Compare using binary_op, to also support pointer values
                     let const_int = Scalar::from_uint(const_int, discr.layout.size);
                     let (res, _) = self.binary_op(mir::BinOp::Eq,
-                        discr.to_scalar()?, discr.layout,
-                        const_int, discr.layout,
+                        discr,
+                        ImmTy::from_scalar(const_int, discr.layout),
                     )?;
                     if res.to_bool()? {
                         target_block = targets[index];
diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs
@@ -370,13 +370,12 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> {
 
                 let (arg, _) = self.eval_operand(arg, source_info)?;
                 let val = self.use_ecx(source_info, |this| {
-                    let prim = this.ecx.read_scalar(arg)?.not_undef()?;
+                    let prim = this.ecx.read_immediate(arg)?;
                     match op {
                         UnOp::Neg => {
                             // Need to do overflow check here: For actual CTFE, MIR
                             // generation emits code that does this before calling the op.
-                            let size = arg.layout.size;
-                            if prim.to_bits(size)? == (1 << (size.bits() - 1)) {
+                            if prim.to_bits()? == (1 << (prim.layout.size.bits() - 1)) {
                                 return err!(OverflowNeg);
                             }
                         }
@@ -385,7 +384,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> {
                         }
                     }
                     // Now run the actual operation.
-                    this.ecx.unary_op(op, prim, arg.layout)
+                    this.ecx.unary_op(op, prim)
                 })?;
                 let res = ImmTy {
                     imm: Immediate::Scalar(val.into()),
@@ -446,7 +445,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> {
                 })?;
                 trace!("const evaluating {:?} for {:?} and {:?}", op, left, right);
                 let (val, overflow) = self.use_ecx(source_info, |this| {
-                    this.ecx.binary_op_imm(op, l, r)
+                    this.ecx.binary_op(op, l, r)
                 })?;
                 let val = if let Rvalue::CheckedBinaryOp(..) = *rvalue {
                     Immediate::ScalarPair(

Original file line number	Diff line number	Diff line change
`@@ -176,7 +176,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M>`
`176`	`176`	`UnaryOp(un_op, ref operand) => {`
`177`	`177`	`// The operand always has the same type as the result.`
`178`	`178`	`let val = self.read_immediate(self.eval_operand(operand, Some(dest.layout))?)?;`
`179`		`- let val = self.unary_op(un_op, val.to_scalar()?, dest.layout)?;`
	`179`	`+ let val = self.unary_op(un_op, val)?;`
`180`	`180`	`self.write_scalar(val, dest)?;`
`181`	`181`	`}`
`182`	`182`