Skip to content

Commit

Permalink
make ptr_op finally reponsible for all ops involving pointers; make V…
Browse files Browse the repository at this point in the history
…alTy constructor private

Also remove public OpTy constructors, but a pub(crate) constructor remains
  • Loading branch information
RalfJung committed Aug 29, 2018
1 parent ec056d5 commit 1d498d5
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 77 deletions.
18 changes: 7 additions & 11 deletions src/librustc_mir/const_eval.rs
Expand Up @@ -288,21 +288,17 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeEvaluator {
)
}

fn try_ptr_op<'a>(
fn ptr_op<'a>(
_ecx: &EvalContext<'a, 'mir, 'tcx, Self>,
_bin_op: mir::BinOp,
left: Scalar,
_left: Scalar,
_left_layout: TyLayout<'tcx>,
right: Scalar,
_right: Scalar,
_right_layout: TyLayout<'tcx>,
) -> EvalResult<'tcx, Option<(Scalar, bool)>> {
if left.is_bits() && right.is_bits() {
Ok(None)
} else {
Err(
ConstEvalError::NeedsRfc("pointer arithmetic or comparison".to_string()).into(),
)
}
) -> EvalResult<'tcx, (Scalar, bool)> {
Err(
ConstEvalError::NeedsRfc("pointer arithmetic or comparison".to_string()).into(),
)
}

fn find_foreign_static<'a>(
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_mir/interpret/cast.rs
Expand Up @@ -48,13 +48,13 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
Misc => {
let src = self.read_value(src)?;
if self.type_is_fat_ptr(src_layout.ty) {
match (src.value, self.type_is_fat_ptr(dest.layout.ty)) {
match (*src, self.type_is_fat_ptr(dest.layout.ty)) {
// pointers to extern types
(Value::Scalar(_),_) |
// slices and trait objects to other slices/trait objects
(Value::ScalarPair(..), true) => {
// No change to value
self.write_value(src.value, dest)?;
self.write_value(*src, dest)?;
}
// slices and trait objects to thin pointers (dropping the metadata)
(Value::ScalarPair(data, _), false) => {
Expand Down
10 changes: 4 additions & 6 deletions src/librustc_mir/interpret/machine.rs
Expand Up @@ -69,20 +69,18 @@ pub trait Machine<'mir, 'tcx>: Clone + Eq + Hash {
def_id: DefId,
) -> EvalResult<'tcx, &'tcx Allocation>;

/// Called for all binary operations except on float types.
///
/// Returns `None` if the operation should be handled by the integer
/// op code in order to share more code between machines
/// Called for all binary operations on integer(-like) types when one operand is a pointer
/// value, and for the `Offset` operation that is inherently about pointers.
///
/// Returns a (value, overflowed) pair if the operation succeeded
fn try_ptr_op<'a>(
fn ptr_op<'a>(
ecx: &EvalContext<'a, 'mir, 'tcx, Self>,
bin_op: mir::BinOp,
left: Scalar,
left_layout: TyLayout<'tcx>,
right: Scalar,
right_layout: TyLayout<'tcx>,
) -> EvalResult<'tcx, Option<(Scalar, bool)>>;
) -> EvalResult<'tcx, (Scalar, bool)>;

/// Heap allocations via the `box` keyword
///
Expand Down
35 changes: 4 additions & 31 deletions src/librustc_mir/interpret/operand.rs
Expand Up @@ -15,7 +15,7 @@ use std::hash::{Hash, Hasher};
use std::convert::TryInto;

use rustc::{mir, ty};
use rustc::ty::layout::{self, Size, Align, LayoutOf, TyLayout, HasDataLayout, IntegerExt};
use rustc::ty::layout::{self, Size, LayoutOf, TyLayout, HasDataLayout, IntegerExt};
use rustc_data_structures::indexed_vec::Idx;

use rustc::mir::interpret::{
Expand Down Expand Up @@ -85,7 +85,7 @@ impl<'tcx> Value {
// as input for binary and cast operations.
#[derive(Copy, Clone, Debug)]
pub struct ValTy<'tcx> {
pub value: Value,
value: Value,
pub layout: TyLayout<'tcx>,
}

Expand All @@ -107,16 +107,6 @@ pub enum Operand {
}

impl Operand {
#[inline]
pub fn from_ptr(ptr: Pointer, align: Align) -> Self {
Operand::Indirect(MemPlace::from_ptr(ptr, align))
}

#[inline]
pub fn from_scalar_value(val: Scalar) -> Self {
Operand::Immediate(Value::Scalar(val.into()))
}

#[inline]
pub fn to_mem_place(self) -> MemPlace {
match self {
Expand All @@ -138,7 +128,7 @@ impl Operand {

#[derive(Copy, Clone, Debug)]
pub struct OpTy<'tcx> {
crate op: Operand, // ideally we'd make this private, but we are not there yet
crate op: Operand, // ideally we'd make this private, but const_prop needs this
pub layout: TyLayout<'tcx>,
}

Expand Down Expand Up @@ -184,23 +174,6 @@ impl<'tcx> PartialEq for OpTy<'tcx> {
}
impl<'tcx> Eq for OpTy<'tcx> {}

impl<'tcx> OpTy<'tcx> {
#[inline]
pub fn from_ptr(ptr: Pointer, align: Align, layout: TyLayout<'tcx>) -> Self {
OpTy { op: Operand::from_ptr(ptr, align), layout }
}

#[inline]
pub fn from_aligned_ptr(ptr: Pointer, layout: TyLayout<'tcx>) -> Self {
OpTy { op: Operand::from_ptr(ptr, layout.align), layout }
}

#[inline]
pub fn from_scalar_value(val: Scalar, layout: TyLayout<'tcx>) -> Self {
OpTy { op: Operand::Immediate(Value::Scalar(val.into())), layout }
}
}

// Use the existing layout if given (but sanity check in debug mode),
// or compute the layout.
#[inline(always)]
Expand Down Expand Up @@ -507,7 +480,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
ConstValue::ByRef(id, alloc, offset) => {
// We rely on mutability being set correctly in that allocation to prevent writes
// where none should happen -- and for `static mut`, we copy on demand anyway.
Ok(Operand::from_ptr(Pointer::new(id, offset), alloc.align))
Ok(Operand::Indirect(MemPlace::from_ptr(Pointer::new(id, offset), alloc.align)))
},
ConstValue::ScalarPair(a, b) =>
Ok(Operand::Immediate(Value::ScalarPair(a.into(), b))),
Expand Down
39 changes: 26 additions & 13 deletions src/librustc_mir/interpret/operator.rs
Expand Up @@ -28,7 +28,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
right: ValTy<'tcx>,
dest: PlaceTy<'tcx>,
) -> EvalResult<'tcx> {
let (val, overflowed) = self.binary_op(op, left, right)?;
let (val, overflowed) = self.binary_op_val(op, left, right)?;
let val = Value::ScalarPair(val.into(), Scalar::from_bool(overflowed).into());
self.write_value(val, dest)
}
Expand All @@ -42,7 +42,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
right: ValTy<'tcx>,
dest: PlaceTy<'tcx>,
) -> EvalResult<'tcx> {
let (val, _overflowed) = self.binary_op(op, left, right)?;
let (val, _overflowed) = self.binary_op_val(op, left, right)?;
self.write_scalar(val, dest)
}
}
Expand Down Expand Up @@ -282,16 +282,31 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
Ok((val, false))
}

/// Convenience wrapper that's useful when keeping the layout together with the
/// value.
#[inline]
pub fn binary_op_val(
&self,
bin_op: mir::BinOp,
left: ValTy<'tcx>,
right: ValTy<'tcx>,
) -> EvalResult<'tcx, (Scalar, 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,
ValTy { value: left, layout: left_layout }: ValTy<'tcx>,
ValTy { value: right, layout: right_layout }: ValTy<'tcx>,
left: Scalar,
left_layout: TyLayout<'tcx>,
right: Scalar,
right_layout: TyLayout<'tcx>,
) -> EvalResult<'tcx, (Scalar, bool)> {
let left = left.to_scalar()?;
let right = right.to_scalar()?;

trace!("Running binary op {:?}: {:?} ({:?}), {:?} ({:?})",
bin_op, left, left_layout.ty, right, right_layout.ty);

Expand Down Expand Up @@ -322,15 +337,13 @@ impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
right_layout.ty.is_fn());

// Handle operations that support pointer values
if let Some(handled) =
M::try_ptr_op(self, bin_op, left, left_layout, right, right_layout)?
{
return Ok(handled);
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);
}

// Everything else only works with "proper" bits
let left = left.to_bits(left_layout.size)?;
let right = right.to_bits(right_layout.size)?;
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)
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_mir/interpret/terminator.rs
Expand Up @@ -17,7 +17,7 @@ use rustc_target::spec::abi::Abi;

use rustc::mir::interpret::{EvalResult, PointerArithmetic, EvalErrorKind, Scalar};
use super::{
EvalContext, Machine, Value, OpTy, Place, PlaceTy, ValTy, Operand, StackPopCleanup
EvalContext, Machine, Value, OpTy, Place, PlaceTy, Operand, StackPopCleanup
};

impl<'a, 'mir, 'tcx, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> {
Expand Down Expand Up @@ -61,8 +61,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'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,
ValTy { value: Value::Scalar(const_int.into()), layout: discr.layout }
discr.to_scalar()?, discr.layout,
const_int, discr.layout,
)?;
if res.to_bool()? {
target_block = targets[index];
Expand Down
28 changes: 17 additions & 11 deletions src/librustc_mir/transform/const_prop.rs
Expand Up @@ -22,7 +22,7 @@ use rustc::mir::interpret::{
};
use rustc::ty::{TyCtxt, self, Instance};
use interpret::{EvalContext, CompileTimeEvaluator, eval_promoted, mk_borrowck_eval_cx};
use interpret::{Value, OpTy, MemoryKind};
use interpret::{self, Value, OpTy, MemoryKind};
use transform::{MirPass, MirSource};
use syntax::source_map::{Span, DUMMY_SP};
use rustc::ty::subst::Substs;
Expand Down Expand Up @@ -358,13 +358,15 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> {
Rvalue::Len(_) => None,
Rvalue::NullaryOp(NullOp::SizeOf, ty) => {
type_size_of(self.tcx, self.param_env, ty).and_then(|n| Some((
OpTy::from_scalar_value(
Scalar::Bits {
bits: n as u128,
size: self.tcx.data_layout.pointer_size.bytes() as u8,
},
self.tcx.layout_of(self.param_env.and(self.tcx.types.usize)).ok()?,
),
OpTy {
op: interpret::Operand::Immediate(Value::Scalar(
Scalar::Bits {
bits: n as u128,
size: self.tcx.data_layout.pointer_size.bytes() as u8,
}.into()
)),
layout: self.tcx.layout_of(self.param_env.and(self.tcx.types.usize)).ok()?,
},
span,
)))
}
Expand Down Expand Up @@ -399,7 +401,11 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> {
// Now run the actual operation.
this.ecx.unary_op(op, prim, arg.layout)
})?;
Some((OpTy::from_scalar_value(val, place_layout), span))
let res = OpTy {
op: interpret::Operand::Immediate(Value::Scalar(val.into())),
layout: place_layout,
};
Some((res, span))
}
Rvalue::CheckedBinaryOp(op, ref left, ref right) |
Rvalue::BinaryOp(op, ref left, ref right) => {
Expand Down Expand Up @@ -454,7 +460,7 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> {
})?;
trace!("const evaluating {:?} for {:?} and {:?}", op, left, right);
let (val, overflow) = self.use_ecx(source_info, |this| {
this.ecx.binary_op(op, l, r)
this.ecx.binary_op_val(op, l, r)
})?;
let val = if let Rvalue::CheckedBinaryOp(..) = *rvalue {
Value::ScalarPair(
Expand All @@ -470,7 +476,7 @@ impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> {
Value::Scalar(val.into())
};
let res = OpTy {
op: ::interpret::Operand::Immediate(val),
op: interpret::Operand::Immediate(val),
layout: place_layout,
};
Some((res, span))
Expand Down

0 comments on commit 1d498d5

Please sign in to comment.