Decide signdedness on the layout instead of the type

Author: oli-obk

src/librustc/ty/layout.rs

@@ -794,6 +794,17 @@ impl Abi {
             Abi::Aggregate { sized } => !sized
         }
     }
+
+    /// Returns true if this is a single signed integer scalar
+    pub fn is_signed(&self) -> bool {
+        match *self {
+            Abi::Scalar(ref scal) => match scal.value {
+                Primitive::Int(_, signed) => signed,
+                _ => false,
+            },
+            _ => false,
+        }
+    }
 }
 
 #[derive(PartialEq, Eq, Hash, Debug)]

src/librustc/ty/mod.rs

@@ -1824,7 +1824,7 @@ impl<'a, 'gcx, 'tcx> AdtDef {
     }
 
     #[inline]
-    fn eval_explicit_discr(
+    pub fn eval_explicit_discr(
         &self,
         tcx: TyCtxt<'a, 'gcx, 'tcx>,
         expr_did: DefId,
@@ -1871,15 +1871,29 @@ impl<'a, 'gcx, 'tcx> AdtDef {
                         ty,
                     })
                 }
+            },
+            Ok(&ty::Const {
+                val: ConstVal::Value(other),
+                ..
+            }) => {
+                info!("invalid enum discriminant: {:#?}", other);
+                ::middle::const_val::struct_error(
+                    tcx,
+                    tcx.def_span(expr_did),
+                    "constant evaluation of enum discriminant resulted in non-integer",
+                ).emit();
+                None
             }
-            _ => {
+            Err(err) => {
+                err.report(tcx, tcx.def_span(expr_did), "enum discriminant");
                 if !expr_did.is_local() {
                     span_bug!(tcx.def_span(expr_did),
                         "variant discriminant evaluation succeeded \
                             in its crate but failed locally");
                 }
                 None
             }
+            _ => span_bug!(tcx.def_span(expr_did), "const eval "),
         }
     }
 

src/librustc_mir/interpret/cast.rs

@@ -1,4 +1,5 @@
 use rustc::ty::Ty;
+use rustc::ty::layout::LayoutOf;
 use syntax::ast::{FloatTy, IntTy, UintTy};
 
 use rustc_const_math::ConstFloat;
@@ -35,23 +36,30 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
         src_ty: Ty<'tcx>,
         dest_ty: Ty<'tcx>,
     ) -> EvalResult<'tcx, PrimVal> {
+        let signed = self.layout_of(src_ty)?.abi.is_signed();
+        let v = if signed {
+            self.sign_extend(v, src_ty)?
+        } else {
+            v
+        };
         trace!("cast_from_int: {}, {}, {}", v, src_ty, dest_ty);
         use rustc::ty::TypeVariants::*;
         match dest_ty.sty {
             TyInt(_) | TyUint(_) => {
-                let v = self.sign_extend(v, src_ty)?;
                 let v = self.truncate(v, dest_ty)?;
                 Ok(PrimVal::Bytes(v))
             }
 
-            TyFloat(fty) if src_ty.is_signed() => Ok(PrimVal::Bytes(ConstFloat::from_i128(v as i128, fty).bits)),
+            TyFloat(fty) if signed => Ok(PrimVal::Bytes(ConstFloat::from_i128(v as i128, fty).bits)),
             TyFloat(fty) => Ok(PrimVal::Bytes(ConstFloat::from_u128(v, fty).bits)),
 
             TyChar if v as u8 as u128 == v => Ok(PrimVal::Bytes(v)),
             TyChar => err!(InvalidChar(v)),
 
             // No alignment check needed for raw pointers.  But we have to truncate to target ptr size.
-            TyRawPtr(_) => Ok(PrimVal::Bytes(self.memory.truncate_to_ptr(v).0 as u128)),
+            TyRawPtr(_) => {
+                Ok(PrimVal::Bytes(self.memory.truncate_to_ptr(v).0 as u128))
+            },
 
             // Casts to bool are not permitted by rustc, no need to handle them here.
             _ => err!(Unimplemented(format!("int to {:?} cast", dest_ty))),

src/librustc_mir/interpret/eval_context.rs

@@ -1128,20 +1128,22 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
         dest_align: Align,
         dest_ty: Ty<'tcx>,
     ) -> EvalResult<'tcx> {
-        trace!("write_value_to_ptr: {:#?}", value);
         let layout = self.layout_of(dest_ty)?;
+        trace!("write_value_to_ptr: {:#?}, {}, {:#?}", value, dest_ty, layout);
         match value {
             Value::ByRef(ptr, align) => {
                 self.memory.copy(ptr, align.min(layout.align), dest, dest_align.min(layout.align), layout.size.bytes(), false)
             }
             Value::ByVal(primval) => {
-                match layout.abi {
-                    layout::Abi::Scalar(_) => {}
-                    _ if primval.is_undef() => {}
+                let signed = match layout.abi {
+                    layout::Abi::Scalar(ref scal) => match scal.value {
+                        layout::Primitive::Int(_, signed) => signed,
+                        _ => false,
+                    },
+                    _ if primval.is_undef() => false,
                     _ => bug!("write_value_to_ptr: invalid ByVal layout: {:#?}", layout)
-                }
-                // TODO: Do we need signedness?
-                self.memory.write_primval(dest.to_ptr()?, dest_align, primval, layout.size.bytes(), false)
+                };
+                self.memory.write_primval(dest.to_ptr()?, dest_align, primval, layout.size.bytes(), signed)
             }
             Value::ByValPair(a_val, b_val) => {
                 let ptr = dest.to_ptr()?;
@@ -1679,7 +1681,9 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
     }
 
     pub fn sign_extend(&self, value: u128, ty: Ty<'tcx>) -> EvalResult<'tcx, u128> {
-        let size = self.layout_of(ty)?.size.bits();
+        let layout = self.layout_of(ty)?;
+        let size = layout.size.bits();
+        assert!(layout.abi.is_signed());
         // sign extend
         let amt = 128 - size;
         // shift the unsigned value to the left

src/librustc_mir/interpret/memory.rs

@@ -702,19 +702,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'a, 'mir, 'tcx, M> {
                 val.offset as u128
             }
 
-            PrimVal::Bytes(bytes) => {
-                // We need to mask here, or the byteorder crate can die when given a u64 larger
-                // than fits in an integer of the requested size.
-                let mask = match size {
-                    1 => !0u8 as u128,
-                    2 => !0u16 as u128,
-                    4 => !0u32 as u128,
-                    8 => !0u64 as u128,
-                    16 => !0,
-                    n => bug!("unexpected PrimVal::Bytes size: {}", n),
-                };
-                bytes & mask
-            }
+            PrimVal::Bytes(bytes) => bytes,
 
             PrimVal::Undef => {
                 self.mark_definedness(PrimVal::Ptr(ptr).into(), size, false)?;

src/librustc_mir/interpret/operator.rs

@@ -83,21 +83,34 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
         let l = left.to_bytes()?;
         let r = right.to_bytes()?;
 
+        let left_layout = self.layout_of(left_ty)?;
+
         // These ops can have an RHS with a different numeric type.
         if right_kind.is_int() && (bin_op == Shl || bin_op == Shr) {
-            let op: fn(u128, u32) -> (u128, bool) = match bin_op {
-                Shl => u128::overflowing_shl,
-                Shr => u128::overflowing_shr,
-                _ => bug!("it has already been checked that this is a shift op"),
-            };
-            let l = if left_ty.is_signed() {
-                self.sign_extend(l, left_ty)?
+            let signed = left_layout.abi.is_signed();
+            let mut r = r as u32;
+            let size = left_layout.size.bits() as u32;
+            let oflo = r > size;
+            if oflo {
+                r %= size;
+            }
+            let result = if signed {
+                let l = self.sign_extend(l, left_ty)? as i128;
+                let result = match bin_op {
+                    Shl => l << r,
+                    Shr => l >> r,
+                    _ => bug!("it has already been checked that this is a shift op"),
+                };
+                result as u128
             } else {
-                l
+                match bin_op {
+                    Shl => l << r,
+                    Shr => l >> r,
+                    _ => bug!("it has already been checked that this is a shift op"),
+                }
             };
-            let (result, oflo) = op(l, r as u32);
             let truncated = self.truncate(result, left_ty)?;
-            return Ok((PrimVal::Bytes(truncated), oflo || truncated != result));
+            return Ok((PrimVal::Bytes(truncated), oflo));
         }
 
         if left_kind != right_kind {
@@ -137,7 +150,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
             }
         };
 
-        if left_ty.is_signed() {
+        if left_layout.abi.is_signed() {
             let op: Option<fn(&i128, &i128) -> bool> = match bin_op {
                 Lt => Some(i128::lt),
                 Le => Some(i128::le),
@@ -162,7 +175,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
             if let Some(op) = op {
                 let l128 = self.sign_extend(l, left_ty)? as i128;
                 let r = self.sign_extend(r, right_ty)? as i128;
-                let size = self.layout_of(left_ty)?.size.bits();
+                let size = left_layout.size.bits();
                 match bin_op {
                     Rem | Div => {
                         // int_min / -1

src/librustc_typeck/collect.rs

@@ -28,17 +28,14 @@ use astconv::{AstConv, Bounds};
 use lint;
 use constrained_type_params as ctp;
 use middle::lang_items::SizedTraitLangItem;
-use middle::const_val::ConstVal;
 use middle::resolve_lifetime as rl;
 use rustc::mir::mono::Linkage;
-use rustc::traits::Reveal;
 use rustc::ty::subst::Substs;
 use rustc::ty::{ToPredicate, ReprOptions};
 use rustc::ty::{self, AdtKind, ToPolyTraitRef, Ty, TyCtxt};
 use rustc::ty::maps::Providers;
 use rustc::ty::util::IntTypeExt;
 use rustc::util::nodemap::{FxHashSet, FxHashMap};
-use rustc::mir::interpret::{GlobalId, Value, PrimVal};
 use rustc::ty::util::Discr;
 
 use syntax::{abi, ast};
@@ -511,7 +508,6 @@ fn convert_variant_ctor<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
 fn convert_enum_variant_types<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                                         def_id: DefId,
                                         variants: &[hir::Variant]) {
-    let param_env = ty::ParamEnv::empty(Reveal::UserFacing);
     let def = tcx.adt_def(def_id);
     let repr_type = def.repr.discr_type();
     let initial = repr_type.initial_discriminant(tcx);
@@ -522,33 +518,7 @@ fn convert_enum_variant_types<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         let wrapped_discr = prev_discr.map_or(initial, |d| d.wrap_incr(tcx));
         prev_discr = Some(if let Some(e) = variant.node.disr_expr {
             let expr_did = tcx.hir.local_def_id(e.node_id);
-            let substs = Substs::identity_for_item(tcx, expr_did);
-            let instance = ty::Instance::new(expr_did, substs);
-            let global_id = GlobalId {
-                instance,
-                promoted: None
-            };
-            let result = tcx.at(variant.span).const_eval(param_env.and(global_id));
-
-            // enum variant evaluation happens before the global constant check
-            // so we need to report the real error
-            if let Err(ref err) = result {
-                err.report(tcx, variant.span, "enum discriminant");
-            }
-
-            match result {
-                // FIXME: just use `to_raw_bits` here?
-                Ok(&ty::Const {
-                    val: ConstVal::Value(Value::ByVal(PrimVal::Bytes(b))),
-                    ..
-                }) => {
-                    Some(Discr {
-                        val: b,
-                        ty: initial.ty,
-                    })
-                }
-                _ => None
-            }
+            def.eval_explicit_discr(tcx, expr_did)
         } else if let Some(discr) = repr_type.disr_incr(tcx, prev_discr) {
             Some(discr)
         } else {