Skip to content

Commit

Permalink
interpret: make some large types not Copy
Browse files Browse the repository at this point in the history
  • Loading branch information
RalfJung committed Jul 18, 2022
1 parent 388971b commit 213a25d
Show file tree
Hide file tree
Showing 9 changed files with 45 additions and 39 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
body,
loc: Err(body.span), // Span used for errors caused during preamble.
return_to_block,
return_place: *return_place,
return_place: return_place.clone(),
// empty local array, we fill it in below, after we are inside the stack frame and
// all methods actually know about the frame
locals: IndexVec::new(),
Expand Down Expand Up @@ -795,7 +795,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let op = self
.local_to_op(self.frame(), mir::RETURN_PLACE, None)
.expect("return place should always be live");
let dest = self.frame().return_place;
let dest = self.frame().return_place.clone();
let err = self.copy_op(&op, &dest, /*allow_transmute*/ true);
trace!("return value: {:?}", self.dump_place(*dest));
// We delay actually short-circuiting on this error until *after* the stack frame is
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_const_eval/src/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,8 +457,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

for i in 0..dest_len {
let place = self.mplace_index(&dest, i)?;
let value =
if i == index { *elem } else { self.mplace_index(&input, i)?.into() };
let value = if i == index {
elem.clone()
} else {
self.mplace_index(&input, i)?.into()
};
self.copy_op(&value, &place.into(), /*allow_transmute*/ false)?;
}
}
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ impl<'tcx, Tag: Provenance> Immediate<Tag> {

// ScalarPair needs a type to interpret, so we often have an immediate and a type together
// as input for binary and cast operations.
#[derive(Copy, Clone, Debug)]
#[derive(Clone, Debug)]
pub struct ImmTy<'tcx, Tag: Provenance = AllocId> {
imm: Immediate<Tag>,
pub layout: TyAndLayout<'tcx>,
Expand Down Expand Up @@ -187,7 +187,10 @@ pub enum Operand<Tag: Provenance = AllocId> {
Indirect(MemPlace<Tag>),
}

#[derive(Copy, Clone, Debug)]
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
rustc_data_structures::static_assert_size!(Operand, 64);

#[derive(Clone, Debug)]
pub struct OpTy<'tcx, Tag: Provenance = AllocId> {
op: Operand<Tag>, // Keep this private; it helps enforce invariants.
pub layout: TyAndLayout<'tcx>,
Expand Down
32 changes: 16 additions & 16 deletions compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,21 @@ pub struct MemPlace<Tag: Provenance = AllocId> {
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
rustc_data_structures::static_assert_size!(MemPlace, 40);

/// A MemPlace with its layout. Constructing it is only possible in this module.
#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)]
pub struct MPlaceTy<'tcx, Tag: Provenance = AllocId> {
mplace: MemPlace<Tag>,
pub layout: TyAndLayout<'tcx>,
/// rustc does not have a proper way to represent the type of a field of a `repr(packed)` struct:
/// it needs to have a different alignment than the field type would usually have.
/// So we represent this here with a separate field that "overwrites" `layout.align`.
/// This means `layout.align` should never be used for a `MPlaceTy`!
pub align: Align,
}

#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
rustc_data_structures::static_assert_size!(MPlaceTy<'_>, 64);

#[derive(Copy, Clone, Debug)]
pub enum Place<Tag: Provenance = AllocId> {
/// A place referring to a value allocated in the `Memory` system.
Expand All @@ -72,7 +87,7 @@ pub enum Place<Tag: Provenance = AllocId> {
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
rustc_data_structures::static_assert_size!(Place, 48);

#[derive(Copy, Clone, Debug)]
#[derive(Clone, Debug)]
pub struct PlaceTy<'tcx, Tag: Provenance = AllocId> {
place: Place<Tag>, // Keep this private; it helps enforce invariants.
pub layout: TyAndLayout<'tcx>,
Expand All @@ -94,21 +109,6 @@ impl<'tcx, Tag: Provenance> std::ops::Deref for PlaceTy<'tcx, Tag> {
}
}

/// A MemPlace with its layout. Constructing it is only possible in this module.
#[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)]
pub struct MPlaceTy<'tcx, Tag: Provenance = AllocId> {
mplace: MemPlace<Tag>,
pub layout: TyAndLayout<'tcx>,
/// rustc does not have a proper way to represent the type of a field of a `repr(packed)` struct:
/// it needs to have a different alignment than the field type would usually have.
/// So we represent this here with a separate field that "overwrites" `layout.align`.
/// This means `layout.align` should never be used for a `MPlaceTy`!
pub align: Align,
}

#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
rustc_data_structures::static_assert_size!(MPlaceTy<'_>, 64);

impl<'tcx, Tag: Provenance> std::ops::Deref for MPlaceTy<'tcx, Tag> {
type Target = MemPlace<Tag>;
#[inline(always)]
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_const_eval/src/interpret/projection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ where
variant: VariantIdx,
) -> InterpResult<'tcx, PlaceTy<'tcx, M::PointerTag>> {
// Downcast just changes the layout
let mut base = *base;
let mut base = base.clone();
base.layout = base.layout.for_variant(self, variant);
Ok(base)
}
Expand All @@ -168,7 +168,7 @@ where
variant: VariantIdx,
) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
// Downcast just changes the layout
let mut base = *base;
let mut base = base.clone();
base.layout = base.layout.for_variant(self, variant);
Ok(base)
}
Expand Down Expand Up @@ -350,7 +350,7 @@ where
use rustc_middle::mir::ProjectionElem::*;
Ok(match proj_elem {
OpaqueCast(ty) => {
let mut place = *base;
let mut place = base.clone();
place.layout = self.layout_of(ty)?;
place
}
Expand Down Expand Up @@ -379,7 +379,7 @@ where
use rustc_middle::mir::ProjectionElem::*;
Ok(match proj_elem {
OpaqueCast(ty) => {
let mut op = *base;
let mut op = base.clone();
op.layout = self.layout_of(ty)?;
op
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/interpret/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
trace!("eval_fn_call: Will pass last argument by untupling");
Cow::from(
args.iter()
.map(|&a| Ok(a))
.map(|a| Ok(a.clone()))
.chain(
(0..untuple_arg.layout.fields.count())
.map(|i| self.operand_field(untuple_arg, i)),
Expand Down Expand Up @@ -525,7 +525,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// We have to implement all "object safe receivers". So we have to go search for a
// pointer or `dyn Trait` type, but it could be wrapped in newtypes. So recursively
// unwrap those newtypes until we are there.
let mut receiver = args[0];
let mut receiver = args[0].clone();
let receiver_place = loop {
match receiver.layout.ty.kind() {
ty::Ref(..) | ty::RawPtr(..) => break self.deref_operand(&receiver)?,
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_const_eval/src/interpret/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use super::{InterpCx, MPlaceTy, Machine, OpTy, PlaceTy};
/// A thing that we can project into, and that has a layout.
/// This wouldn't have to depend on `Machine` but with the current type inference,
/// that's just more convenient to work with (avoids repeating all the `Machine` bounds).
pub trait Value<'mir, 'tcx, M: Machine<'mir, 'tcx>>: Copy {
pub trait Value<'mir, 'tcx, M: Machine<'mir, 'tcx>>: Sized {
/// Gets this value's layout.
fn layout(&self) -> TyAndLayout<'tcx>;

Expand Down Expand Up @@ -54,7 +54,7 @@ pub trait Value<'mir, 'tcx, M: Machine<'mir, 'tcx>>: Copy {
/// A thing that we can project into given *mutable* access to `ecx`, and that has a layout.
/// This wouldn't have to depend on `Machine` but with the current type inference,
/// that's just more convenient to work with (avoids repeating all the `Machine` bounds).
pub trait ValueMut<'mir, 'tcx, M: Machine<'mir, 'tcx>>: Copy {
pub trait ValueMut<'mir, 'tcx, M: Machine<'mir, 'tcx>>: Sized {
/// Gets this value's layout.
fn layout(&self) -> TyAndLayout<'tcx>;

Expand Down Expand Up @@ -106,12 +106,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> Value<'mir, 'tcx, M> for OpTy<'tc
&self,
_ecx: &InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
Ok(*self)
Ok(self.clone())
}

#[inline(always)]
fn from_op(op: &OpTy<'tcx, M::PointerTag>) -> Self {
*op
op.clone()
}

#[inline(always)]
Expand Down Expand Up @@ -146,20 +146,20 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueMut<'mir, 'tcx, M>
&self,
_ecx: &InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
Ok(*self)
Ok(self.clone())
}

#[inline(always)]
fn to_op_for_proj(
&self,
_ecx: &mut InterpCx<'mir, 'tcx, M>,
) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
Ok(*self)
Ok(self.clone())
}

#[inline(always)]
fn from_op(op: &OpTy<'tcx, M::PointerTag>) -> Self {
*op
op.clone()
}

#[inline(always)]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
let l = self.use_ecx(|this| this.ecx.read_immediate(&this.ecx.eval_operand(left, None)?));
// Check for exceeding shifts *even if* we cannot evaluate the LHS.
if op == BinOp::Shr || op == BinOp::Shl {
let r = r?;
let r = r.clone()?;
// We need the type of the LHS. We cannot use `place_layout` as that is the type
// of the result, which for checked binops is not the same!
let left_ty = left.ty(self.local_decls, self.tcx);
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_mir_transform/src/const_prop_lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
});
// Check for exceeding shifts *even if* we cannot evaluate the LHS.
if op == BinOp::Shr || op == BinOp::Shl {
let r = r?;
let r = r.clone()?;
// We need the type of the LHS. We cannot use `place_layout` as that is the type
// of the result, which for checked binops is not the same!
let left_ty = left.ty(self.local_decls, self.tcx);
Expand Down Expand Up @@ -616,10 +616,10 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
}
}

if let (Some(l), Some(r)) = (&l, &r) {
if let (Some(l), Some(r)) = (l, r) {
// The remaining operators are handled through `overflowing_binary_op`.
if self.use_ecx(source_info, |this| {
let (_res, overflow, _ty) = this.ecx.overflowing_binary_op(op, l, r)?;
let (_res, overflow, _ty) = this.ecx.overflowing_binary_op(op, &l, &r)?;
Ok(overflow)
})? {
self.report_assert_as_lint(
Expand Down

0 comments on commit 213a25d

Please sign in to comment.