Skip to content

Commit

Permalink
Rollup merge of #99259 - RalfJung:visit-a-place, r=oli-obk
Browse files Browse the repository at this point in the history
interpret/visitor: support visiting with a PlaceTy

Finally we can visit a `PlaceTy` in a way that will only do `force_allocation` when needed ti visit a field. :)

r? `@oli-obk`
  • Loading branch information
matthiaskrgr authored Jul 16, 2022
2 parents 984ef42 + c4cb043 commit fa298be
Show file tree
Hide file tree
Showing 6 changed files with 272 additions and 50 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_const_eval/src/const_eval/valtrees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ fn valtree_into_mplace<'tcx>(

let offset = place_adjusted.layout.fields.offset(i);
place
.offset(
.offset_with_meta(
offset,
MemPlaceMeta::Meta(Scalar::from_machine_usize(
num_elems as u64,
Expand Down
14 changes: 12 additions & 2 deletions compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,15 +297,15 @@ impl<'tcx, Tag: Provenance> OpTy<'tcx, Tag> {
}
}

pub fn offset(
pub fn offset_with_meta(
&self,
offset: Size,
meta: MemPlaceMeta<Tag>,
layout: TyAndLayout<'tcx>,
cx: &impl HasDataLayout,
) -> InterpResult<'tcx, Self> {
match self.try_as_mplace() {
Ok(mplace) => Ok(mplace.offset(offset, meta, layout, cx)?.into()),
Ok(mplace) => Ok(mplace.offset_with_meta(offset, meta, layout, cx)?.into()),
Err(imm) => {
assert!(
matches!(*imm, Immediate::Uninit),
Expand All @@ -317,6 +317,16 @@ impl<'tcx, Tag: Provenance> OpTy<'tcx, Tag> {
}
}
}

pub fn offset(
&self,
offset: Size,
layout: TyAndLayout<'tcx>,
cx: &impl HasDataLayout,
) -> InterpResult<'tcx, Self> {
assert!(!layout.is_unsized());
self.offset_with_meta(offset, MemPlaceMeta::None, layout, cx)
}
}

impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Expand Down
16 changes: 13 additions & 3 deletions compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ impl<Tag: Provenance> MemPlace<Tag> {
}

#[inline]
pub fn offset<'tcx>(
pub fn offset_with_meta<'tcx>(
self,
offset: Size,
meta: MemPlaceMeta<Tag>,
Expand Down Expand Up @@ -199,20 +199,30 @@ impl<'tcx, Tag: Provenance> MPlaceTy<'tcx, Tag> {
}

#[inline]
pub fn offset(
pub fn offset_with_meta(
&self,
offset: Size,
meta: MemPlaceMeta<Tag>,
layout: TyAndLayout<'tcx>,
cx: &impl HasDataLayout,
) -> InterpResult<'tcx, Self> {
Ok(MPlaceTy {
mplace: self.mplace.offset(offset, meta, cx)?,
mplace: self.mplace.offset_with_meta(offset, meta, cx)?,
align: self.align.restrict_for_offset(offset),
layout,
})
}

pub fn offset(
&self,
offset: Size,
layout: TyAndLayout<'tcx>,
cx: &impl HasDataLayout,
) -> InterpResult<'tcx, Self> {
assert!(!layout.is_unsized());
self.offset_with_meta(offset, MemPlaceMeta::None, layout, cx)
}

#[inline]
pub fn from_aligned_ptr(ptr: Pointer<Option<Tag>>, layout: TyAndLayout<'tcx>) -> Self {
MPlaceTy { mplace: MemPlace::from_ptr(ptr), layout, align: layout.align.abi }
Expand Down
12 changes: 5 additions & 7 deletions compiler/rustc_const_eval/src/interpret/projection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ where

// We do not look at `base.layout.align` nor `field_layout.align`, unlike
// codegen -- mostly to see if we can get away with that
base.offset(offset, meta, field_layout, self)
base.offset_with_meta(offset, meta, field_layout, self)
}

/// Gets the place of a field inside the place, and also the field's type.
Expand Down Expand Up @@ -193,9 +193,7 @@ where
let offset = stride * index; // `Size` multiplication
// All fields have the same layout.
let field_layout = base.layout.field(self, 0);
assert!(!field_layout.is_unsized());

base.offset(offset, MemPlaceMeta::None, field_layout, self)
base.offset(offset, field_layout, self)
}
_ => span_bug!(
self.cur_span(),
Expand All @@ -215,10 +213,10 @@ where
let abi::FieldsShape::Array { stride, .. } = base.layout.fields else {
span_bug!(self.cur_span(), "operand_array_fields: expected an array layout");
};
let layout = base.layout.field(self, 0);
let field_layout = base.layout.field(self, 0);
let dl = &self.tcx.data_layout;
// `Size` multiplication
Ok((0..len).map(move |i| base.offset(stride * i, MemPlaceMeta::None, layout, dl)))
Ok((0..len).map(move |i| base.offset(stride * i, field_layout, dl)))
}

/// Index into an array.
Expand Down Expand Up @@ -326,7 +324,7 @@ where
}
};
let layout = self.layout_of(ty)?;
base.offset(from_offset, meta, layout, self)
base.offset_with_meta(from_offset, meta, layout, self)
}

pub fn place_subslice(
Expand Down
6 changes: 4 additions & 2 deletions compiler/rustc_const_eval/src/interpret/validity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,8 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
self.visit_scalar(scalar, scalar_layout)?;
}
Abi::ScalarPair(a_layout, b_layout) => {
// We would validate these things as we descend into the fields,
// There is no `rustc_layout_scalar_valid_range_start` for pairs, so
// we would validate these things as we descend into the fields,
// but that can miss bugs in layout computation. Layout computation
// is subtle due to enums having ScalarPair layout, where one field
// is the discriminant.
Expand All @@ -867,7 +868,8 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M>
}
Abi::Vector { .. } => {
// No checks here, we assume layout computation gets this right.
// (This is harder to check since Miri does not represent these as `Immediate`.)
// (This is harder to check since Miri does not represent these as `Immediate`. We
// also cannot use field projections since this might be a newtype around a vector.)
}
Abi::Aggregate { .. } => {
// Nothing to do.
Expand Down
Loading

0 comments on commit fa298be

Please sign in to comment.