Skip to content

Commit

Permalink
make both unary_op and binary_op fully typed, including a return type
Browse files Browse the repository at this point in the history
  • Loading branch information
@RalfJung
RalfJung committed Aug 17, 2019
1 parent 0a2a517 commit 3edf099
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 69 deletions.
5 changes: 2 additions & 3 deletions src/librustc_mir/const_eval.rs
View file
Expand Up @@ -11,9 +11,8 @@ use rustc::hir::def::DefKind;
use rustc::hir::def_id::DefId;
use rustc::mir::interpret::{ConstEvalErr, ErrorHandled, ScalarMaybeUndef};
use rustc::mir;
use rustc::ty::{self, TyCtxt};
use rustc::ty::{self, Ty, TyCtxt, subst::Subst};
use rustc::ty::layout::{self, LayoutOf, VariantIdx};
use rustc::ty::subst::Subst;
use rustc::traits::Reveal;
use rustc_data_structures::fx::FxHashMap;

Expand Down Expand Up @@ -415,7 +414,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
_bin_op: mir::BinOp,
_left: ImmTy<'tcx>,
_right: ImmTy<'tcx>,
) -> InterpResult<'tcx, (Scalar, bool)> {
) -> InterpResult<'tcx, (Scalar, bool, Ty<'tcx>)> {
Err(
ConstEvalError::NeedsRfc("pointer arithmetic or comparison".to_string()).into(),
)
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/intrinsics.rs
View file
Expand Up @@ -137,7 +137,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let l = self.read_immediate(args[0])?;
let r = self.read_immediate(args[1])?;
let is_add = intrinsic_name == "saturating_add";
let (val, overflowed) = self.binary_op(if is_add {
let (val, overflowed, _ty) = self.overflowing_binary_op(if is_add {
BinOp::Add
} else {
BinOp::Sub
Expand Down Expand Up @@ -184,7 +184,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
"unchecked_shr" => BinOp::Shr,
_ => bug!("Already checked for int ops")
};
let (val, overflowed) = self.binary_op(bin_op, l, r)?;
let (val, overflowed, _ty) = self.overflowing_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)?;
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/machine.rs
View file
Expand Up @@ -7,7 +7,7 @@ use std::hash::Hash;

use rustc::hir::def_id::DefId;
use rustc::mir;
use rustc::ty::{self, TyCtxt};
use rustc::ty::{self, Ty, TyCtxt};

use super::{
Allocation, AllocId, InterpResult, Scalar, AllocationExtra,
Expand Down Expand Up @@ -176,7 +176,7 @@ pub trait Machine<'mir, 'tcx>: Sized {
bin_op: mir::BinOp,
left: ImmTy<'tcx, Self::PointerTag>,
right: ImmTy<'tcx, Self::PointerTag>,
) -> InterpResult<'tcx, (Scalar<Self::PointerTag>, bool)>;
) -> InterpResult<'tcx, (Scalar<Self::PointerTag>, bool, Ty<'tcx>)>;

/// Heap allocations via the `box` keyword.
fn box_alloc(
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/operand.rs
View file
Expand Up @@ -108,7 +108,7 @@ impl<'tcx, Tag> Immediate<Tag> {
// as input for binary and cast operations.
#[derive(Copy, Clone, Debug)]
pub struct ImmTy<'tcx, Tag=()> {
pub imm: Immediate<Tag>,
pub(crate) imm: Immediate<Tag>,
pub layout: TyLayout<'tcx>,
}

Expand Down Expand Up @@ -155,7 +155,7 @@ impl<Tag> Operand<Tag> {

#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
pub struct OpTy<'tcx, Tag=()> {
op: Operand<Tag>,
op: Operand<Tag>, // Keep this private, it helps enforce invariants
pub layout: TyLayout<'tcx>,
}

Expand Down
123 changes: 74 additions & 49 deletions src/librustc_mir/interpret/operator.rs
View file
@@ -1,5 +1,5 @@
use rustc::mir;
use rustc::ty::{self, layout::TyLayout};
use rustc::ty::{self, Ty, layout::{TyLayout, LayoutOf}};
use syntax::ast::FloatTy;
use rustc_apfloat::Float;
use rustc::mir::interpret::{InterpResult, Scalar};
Expand All @@ -17,7 +17,12 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
right: ImmTy<'tcx, M::PointerTag>,
dest: PlaceTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx> {
let (val, overflowed) = self.binary_op(op, left, right)?;
let (val, overflowed, ty) = self.overflowing_binary_op(op, left, right)?;
debug_assert_eq!(
self.tcx.intern_tup(&[ty, self.tcx.types.bool]),
dest.layout.ty,
"type mismatch for result of {:?}", op,
);
let val = Immediate::ScalarPair(val.into(), Scalar::from_bool(overflowed).into());
self.write_immediate(val, dest)
}
Expand All @@ -31,7 +36,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
right: ImmTy<'tcx, M::PointerTag>,
dest: PlaceTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx> {
let (val, _overflowed) = self.binary_op(op, left, right)?;
let (val, _overflowed, ty) = self.overflowing_binary_op(op, left, right)?;
assert_eq!(ty, dest.layout.ty, "type mismatch for result of {:?}", op);
self.write_scalar(val, dest)
}
}
Expand All @@ -42,7 +48,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
bin_op: mir::BinOp,
l: char,
r: char,
) -> (Scalar<M::PointerTag>, bool) {
) -> (Scalar<M::PointerTag>, bool, Ty<'tcx>) {
use rustc::mir::BinOp::*;

let res = match bin_op {
Expand All @@ -54,15 +60,15 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Ge => l >= r,
_ => bug!("Invalid operation on char: {:?}", bin_op),
};
return (Scalar::from_bool(res), false);
return (Scalar::from_bool(res), false, self.tcx.types.bool);
}

fn binary_bool_op(
&self,
bin_op: mir::BinOp,
l: bool,
r: bool,
) -> (Scalar<M::PointerTag>, bool) {
) -> (Scalar<M::PointerTag>, bool, Ty<'tcx>) {
use rustc::mir::BinOp::*;

let res = match bin_op {
Expand All @@ -77,32 +83,33 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
BitXor => l ^ r,
_ => bug!("Invalid operation on bool: {:?}", bin_op),
};
return (Scalar::from_bool(res), false);
return (Scalar::from_bool(res), false, self.tcx.types.bool);
}

fn binary_float_op<F: Float + Into<Scalar<M::PointerTag>>>(
&self,
bin_op: mir::BinOp,
ty: Ty<'tcx>,
l: F,
r: F,
) -> (Scalar<M::PointerTag>, bool) {
) -> (Scalar<M::PointerTag>, bool, Ty<'tcx>) {
use rustc::mir::BinOp::*;

let val = match bin_op {
Eq => Scalar::from_bool(l == r),
Ne => Scalar::from_bool(l != r),
Lt => Scalar::from_bool(l < r),
Le => Scalar::from_bool(l <= r),
Gt => Scalar::from_bool(l > r),
Ge => Scalar::from_bool(l >= r),
Add => (l + r).value.into(),
Sub => (l - r).value.into(),
Mul => (l * r).value.into(),
Div => (l / r).value.into(),
Rem => (l % r).value.into(),
let (val, ty) = match bin_op {
Eq => (Scalar::from_bool(l == r), self.tcx.types.bool),
Ne => (Scalar::from_bool(l != r), self.tcx.types.bool),
Lt => (Scalar::from_bool(l < r), self.tcx.types.bool),
Le => (Scalar::from_bool(l <= r), self.tcx.types.bool),
Gt => (Scalar::from_bool(l > r), self.tcx.types.bool),
Ge => (Scalar::from_bool(l >= r), self.tcx.types.bool),
Add => ((l + r).value.into(), ty),
Sub => ((l - r).value.into(), ty),
Mul => ((l * r).value.into(), ty),
Div => ((l / r).value.into(), ty),
Rem => ((l % r).value.into(), ty),
_ => bug!("invalid float op: `{:?}`", bin_op),
};
return (val, false);
return (val, false, ty);
}

fn binary_int_op(
Expand All @@ -113,7 +120,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
left_layout: TyLayout<'tcx>,
r: u128,
right_layout: TyLayout<'tcx>,
) -> InterpResult<'tcx, (Scalar<M::PointerTag>, bool)> {
) -> InterpResult<'tcx, (Scalar<M::PointerTag>, bool, Ty<'tcx>)> {
use rustc::mir::BinOp::*;

// Shift ops can have an RHS with a different numeric type.
Expand Down Expand Up @@ -142,7 +149,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
};
let truncated = self.truncate(result, left_layout);
return Ok((Scalar::from_uint(truncated, size), oflo));
return Ok((Scalar::from_uint(truncated, size), oflo, left_layout.ty));
}

// For the remaining ops, the types must be the same on both sides
Expand All @@ -167,7 +174,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
if let Some(op) = op {
let l = self.sign_extend(l, left_layout) as i128;
let r = self.sign_extend(r, right_layout) as i128;
return Ok((Scalar::from_bool(op(&l, &r)), false));
return Ok((Scalar::from_bool(op(&l, &r)), false, self.tcx.types.bool));
}
let op: Option<fn(i128, i128) -> (i128, bool)> = match bin_op {
Div if r == 0 => throw_panic!(DivisionByZero),
Expand All @@ -187,7 +194,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Rem | Div => {
// int_min / -1
if r == -1 && l == (1 << (size.bits() - 1)) {
return Ok((Scalar::from_uint(l, size), true));
return Ok((Scalar::from_uint(l, size), true, left_layout.ty));
}
},
_ => {},
Expand All @@ -202,25 +209,24 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// this may be out-of-bounds for the result type, so we have to truncate ourselves
let result = result as u128;
let truncated = self.truncate(result, left_layout);
return Ok((Scalar::from_uint(truncated, size), oflo));
return Ok((Scalar::from_uint(truncated, size), oflo, left_layout.ty));
}
}

let size = left_layout.size;

// only ints left
let val = match bin_op {
Eq => Scalar::from_bool(l == r),
Ne => Scalar::from_bool(l != r),
let (val, ty) = match bin_op {
Eq => (Scalar::from_bool(l == r), self.tcx.types.bool),
Ne => (Scalar::from_bool(l != r), self.tcx.types.bool),

Lt => Scalar::from_bool(l < r),
Le => Scalar::from_bool(l <= r),
Gt => Scalar::from_bool(l > r),
Ge => Scalar::from_bool(l >= r),
Lt => (Scalar::from_bool(l < r), self.tcx.types.bool),
Le => (Scalar::from_bool(l <= r), self.tcx.types.bool),
Gt => (Scalar::from_bool(l > r), self.tcx.types.bool),
Ge => (Scalar::from_bool(l >= r), self.tcx.types.bool),

BitOr => Scalar::from_uint(l | r, size),
BitAnd => Scalar::from_uint(l & r, size),
BitXor => Scalar::from_uint(l ^ r, size),
BitOr => (Scalar::from_uint(l | r, size), left_layout.ty),
BitAnd => (Scalar::from_uint(l & r, size), left_layout.ty),
BitXor => (Scalar::from_uint(l ^ r, size), left_layout.ty),

Add | Sub | Mul | Rem | Div => {
debug_assert!(!left_layout.abi.is_signed());
Expand All @@ -236,7 +242,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
};
let (result, oflo) = op(l, r);
let truncated = self.truncate(result, left_layout);
return Ok((Scalar::from_uint(truncated, size), oflo || truncated != result));
return Ok((
Scalar::from_uint(truncated, size),
oflo || truncated != result,
left_layout.ty,
));
}

_ => {
Expand All @@ -250,17 +260,17 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
};

Ok((val, false))
Ok((val, false, ty))
}

/// Returns the result of the specified operation and whether it overflowed.
#[inline]
pub fn binary_op(
/// Returns the result of the specified operation, whether it overflowed, and
/// the result type.
pub fn overflowing_binary_op(
&self,
bin_op: mir::BinOp,
left: ImmTy<'tcx, M::PointerTag>,
right: ImmTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx, (Scalar<M::PointerTag>, bool)> {
) -> InterpResult<'tcx, (Scalar<M::PointerTag>, bool, Ty<'tcx>)> {
trace!("Running binary op {:?}: {:?} ({:?}), {:?} ({:?})",
bin_op, *left, left.layout.ty, *right, right.layout.ty);

Expand All @@ -279,11 +289,14 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
ty::Float(fty) => {
assert_eq!(left.layout.ty, right.layout.ty);
let ty = left.layout.ty;
let left = left.to_scalar()?;
let right = right.to_scalar()?;
Ok(match fty {
FloatTy::F32 => self.binary_float_op(bin_op, left.to_f32()?, right.to_f32()?),
FloatTy::F64 => self.binary_float_op(bin_op, left.to_f64()?, right.to_f64()?),
FloatTy::F32 =>
self.binary_float_op(bin_op, ty, left.to_f32()?, right.to_f32()?),
FloatTy::F64 =>
self.binary_float_op(bin_op, ty, left.to_f64()?, right.to_f64()?),
})
}
_ if left.layout.ty.is_integral() => {
Expand Down Expand Up @@ -312,11 +325,23 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
}

/// Typed version of `checked_binary_op`, returning an `ImmTy`. Also ignores overflows.
#[inline]
pub fn binary_op(
&self,
bin_op: mir::BinOp,
left: ImmTy<'tcx, M::PointerTag>,
right: ImmTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx, ImmTy<'tcx, M::PointerTag>> {
let (val, _overflow, ty) = self.overflowing_binary_op(bin_op, left, right)?;
Ok(ImmTy::from_scalar(val, self.layout_of(ty)?))
}

pub fn unary_op(
&self,
un_op: mir::UnOp,
val: ImmTy<'tcx, M::PointerTag>,
) -> InterpResult<'tcx, Scalar<M::PointerTag>> {
) -> InterpResult<'tcx, ImmTy<'tcx, M::PointerTag>> {
use rustc::mir::UnOp::*;

let layout = val.layout;
Expand All @@ -330,15 +355,15 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Not => !val,
_ => bug!("Invalid bool op {:?}", un_op)
};
Ok(Scalar::from_bool(res))
Ok(ImmTy::from_scalar(Scalar::from_bool(res), self.layout_of(self.tcx.types.bool)?))
}
ty::Float(fty) => {
let res = match (un_op, fty) {
(Neg, FloatTy::F32) => Scalar::from_f32(-val.to_f32()?),
(Neg, FloatTy::F64) => Scalar::from_f64(-val.to_f64()?),
_ => bug!("Invalid float op {:?}", un_op)
};
Ok(res)
Ok(ImmTy::from_scalar(res, layout))
}
_ => {
assert!(layout.ty.is_integral());
Expand All @@ -351,7 +376,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
};
// res needs tuncating
Ok(Scalar::from_uint(self.truncate(res, layout), layout.size))
Ok(ImmTy::from_uint(self.truncate(res, layout), layout))
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/interpret/place.rs
View file
Expand Up @@ -45,7 +45,7 @@ pub enum Place<Tag=(), Id=AllocId> {

#[derive(Copy, Clone, Debug)]
pub struct PlaceTy<'tcx, Tag=()> {
place: Place<Tag>,
place: Place<Tag>, // Keep this private, it helps enforce invariants
pub layout: TyLayout<'tcx>,
}

Expand Down
3 changes: 2 additions & 1 deletion src/librustc_mir/interpret/step.rs
View file
Expand Up @@ -177,7 +177,8 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// 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)?;
self.write_scalar(val, dest)?;
assert_eq!(val.layout, dest.layout, "layout mismatch for result of {:?}", un_op);
self.write_immediate(*val, dest)?;
}

Aggregate(ref kind, ref operands) => {
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/terminator.rs
View file
Expand Up @@ -50,10 +50,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

for (index, &const_int) in values.iter().enumerate() {
// Compare using binary_op, to also support pointer values
let (res, _) = self.binary_op(mir::BinOp::Eq,
let res = self.overflowing_binary_op(mir::BinOp::Eq,
discr,
ImmTy::from_uint(const_int, discr.layout),
)?;
)?.0;
if res.to_bool()? {
target_block = targets[index];
break;
Expand Down

0 comments on commit 3edf099

Please sign in to comment.