Skip to content

Commit

Permalink
Auto merge of #121985 - RalfJung:interpret-return-place, r=<try>
Browse files Browse the repository at this point in the history
interpret: avoid a long-lived PlaceTy in stack frames

`PlaceTy` uses a representation that's not very stable under changes to the stack. I'd feel better if we didn't have one in the long-term machine state.

r? `@oli-obk`
  • Loading branch information
bors committed Mar 4, 2024
2 parents 7606c13 + 8de72e4 commit 96f5a38
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 3 deletions.
7 changes: 4 additions & 3 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ pub struct Frame<'mir, 'tcx, Prov: Provenance = CtfeProvenance, Extra = ()> {

/// The location where the result of the current stack frame should be written to,
/// and its layout in the caller.
pub return_place: PlaceTy<'tcx, Prov>,
pub return_place: MPlaceTy<'tcx, Prov>,

/// The list of locals for this stack frame, stored in order as
/// `[return_ptr, arguments..., variables..., temporaries...]`.
Expand Down Expand Up @@ -777,12 +777,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
trace!("body: {:#?}", body);
let dead_local = LocalState { value: LocalValue::Dead, layout: Cell::new(None) };
let locals = IndexVec::from_elem(dead_local, &body.local_decls);
let return_place = self.force_allocation(return_place)?; // avoid a long-lived `PlaceTy`
// First push a stack frame so we have access to the local args
let pre_frame = Frame {
body,
loc: Right(body.span), // Span used for errors caused during preamble.
return_to_block,
return_place: return_place.clone(),
return_place,
locals,
instance,
tracing_span: SpanGuard::new(),
Expand Down Expand Up @@ -912,7 +913,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
} else {
self.copy_op_allow_transmute(&op, &dest)
};
trace!("return value: {:?}", self.dump_place(&dest));
trace!("return value: {:?}", self.dump_place(&dest.into()));
// We delay actually short-circuiting on this error until *after* the stack frame is
// popped, since we want this error to be attributed to the caller, whose type defines
// this transmute.
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,12 @@ pub(super) enum Place<Prov: Provenance = CtfeProvenance> {
Local { frame: usize, local: mir::Local, offset: Option<Size> },
}

/// An evaluated place, together with its type.
///
/// This may reference a stack frame by its index, so `PlaceTy` should generally not be kept around
/// for longer than a single operation. Popping and then pushing a stack frame can make `PlaceTy`
/// point to the wrong destination. If the interpreter has multiple stacks, stack switching will
/// also invalidate a `PlaceTy`.
#[derive(Clone)]
pub struct PlaceTy<'tcx, Prov: Provenance = CtfeProvenance> {
place: Place<Prov>, // Keep this private; it helps enforce invariants.
Expand Down

0 comments on commit 96f5a38

Please sign in to comment.