Skip to content

Commit

Permalink
Auto merge of rust-lang#98145 - ouz-a:some_branch, r=oli-obk
Browse files Browse the repository at this point in the history
Pull Derefer before ElaborateDrops

_Follow up work to rust-lang#97025 rust-lang#96549 rust-lang#96116 rust-lang#95887 #95649_

This moves `Derefer` before `ElaborateDrops` and creates a new `Rvalue` called `VirtualRef` that allows us to bypass many constraints for `DerefTemp`.

r? `@oli-obk`
  • Loading branch information
bors committed Jul 13, 2022
2 parents ca4e394 + b4c3a2a commit 42bd138
Show file tree
Hide file tree
Showing 43 changed files with 401 additions and 227 deletions.
4 changes: 4 additions & 0 deletions compiler/rustc_borrowck/src/invalidation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,10 @@ impl<'cx, 'tcx> InvalidationGenerator<'cx, 'tcx> {
| Rvalue::ShallowInitBox(ref operand, _ /*ty*/) => {
self.consume_operand(location, operand)
}
Rvalue::CopyForDeref(ref place) => {
let op = &Operand::Copy(*place);
self.consume_operand(location, op);
}

Rvalue::Len(place) | Rvalue::Discriminant(place) => {
let af = match *rvalue {
Expand Down
17 changes: 17 additions & 0 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1236,6 +1236,23 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
| Rvalue::ShallowInitBox(ref operand, _ /*ty*/) => {
self.consume_operand(location, (operand, span), flow_state)
}
Rvalue::CopyForDeref(place) => {
self.access_place(
location,
(place, span),
(Deep, Read(ReadKind::Copy)),
LocalMutationIsAllowed::No,
flow_state,
);

// Finally, check if path was already moved.
self.check_if_path_or_subpath_is_moved(
location,
InitializationRequiringAction::Use,
(place.as_ref(), span),
flow_state,
);
}

Rvalue::Len(place) | Rvalue::Discriminant(place) => {
let af = match *rvalue {
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2269,6 +2269,10 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
Rvalue::Use(operand) | Rvalue::UnaryOp(_, operand) => {
self.check_operand(operand, location);
}
Rvalue::CopyForDeref(place) => {
let op = &Operand::Copy(*place);
self.check_operand(op, location);
}

Rvalue::BinaryOp(_, box (left, right))
| Rvalue::CheckedBinaryOp(_, box (left, right)) => {
Expand Down Expand Up @@ -2299,6 +2303,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
| Rvalue::BinaryOp(..)
| Rvalue::CheckedBinaryOp(..)
| Rvalue::NullaryOp(..)
| Rvalue::CopyForDeref(..)
| Rvalue::UnaryOp(..)
| Rvalue::Discriminant(..) => None,

Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,11 @@ fn codegen_stmt<'tcx>(
let val = codegen_operand(fx, operand);
lval.write_cvalue(fx, val);
}
Rvalue::CopyForDeref(place) => {
let cplace = codegen_place(fx, place);
let val = cplace.to_cvalue(fx);
lval.write_cvalue(fx, val)
}
Rvalue::Ref(_, _, place) | Rvalue::AddressOf(_, place) => {
let place = codegen_place(fx, place);
let ref_ = place.place_ref(fx, lval.layout());
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::traits::*;
use crate::MemFlags;

use rustc_middle::mir;
use rustc_middle::mir::Operand;
use rustc_middle::ty::cast::{CastTy, IntTy};
use rustc_middle::ty::layout::{HasTyCtxt, LayoutOf};
use rustc_middle::ty::{self, adjustment::PointerCast, Instance, Ty, TyCtxt};
Expand Down Expand Up @@ -344,6 +345,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
self.codegen_place_to_pointer(bx, place, mk_ref)
}

mir::Rvalue::CopyForDeref(place) => {
let operand = self.codegen_operand(&mut bx, &Operand::Copy(place));
(bx, operand)
}
mir::Rvalue::AddressOf(mutability, place) => {
let mk_ptr = move |tcx: TyCtxt<'tcx>, ty: Ty<'tcx>| {
tcx.mk_ptr(ty::TypeAndMut { ty, mutbl: mutability })
Expand Down Expand Up @@ -698,6 +703,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
pub fn rvalue_creates_operand(&self, rvalue: &mir::Rvalue<'tcx>, span: Span) -> bool {
match *rvalue {
mir::Rvalue::Ref(..) |
mir::Rvalue::CopyForDeref(..) |
mir::Rvalue::AddressOf(..) |
mir::Rvalue::Len(..) |
mir::Rvalue::Cast(..) | // (*)
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_const_eval/src/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
self.copy_op(&op, &dest, /*allow_transmute*/ false)?;
}

CopyForDeref(ref place) => {
let op = self.eval_place_to_op(*place, Some(dest.layout))?;
self.copy_op(&op, &dest, /* allow_transmute*/ false)?;
}

BinaryOp(bin_op, box (ref left, ref right)) => {
let layout = binop_left_homogeneous(bin_op).then_some(dest.layout);
let left = self.read_immediate(&self.eval_operand(left, layout)?)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
Rvalue::ThreadLocalRef(_) => self.check_op(ops::ThreadLocalAccess),

Rvalue::Use(_)
| Rvalue::CopyForDeref(..)
| Rvalue::Repeat(..)
| Rvalue::Discriminant(..)
| Rvalue::Len(_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,8 @@ where
in_place::<Q, _>(cx, in_local, place.as_ref())
}

Rvalue::CopyForDeref(place) => in_place::<Q, _>(cx, in_local, place.as_ref()),

Rvalue::Use(operand)
| Rvalue::Repeat(operand, _)
| Rvalue::UnaryOp(_, operand)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ where
mir::Rvalue::Cast(..)
| mir::Rvalue::ShallowInitBox(..)
| mir::Rvalue::Use(..)
| mir::Rvalue::CopyForDeref(..)
| mir::Rvalue::ThreadLocalRef(..)
| mir::Rvalue::Repeat(..)
| mir::Rvalue::Len(..)
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_const_eval/src/transform/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,10 @@ impl<'tcx> Validator<'_, 'tcx> {
Rvalue::Use(operand) | Rvalue::Repeat(operand, _) => {
self.validate_operand(operand)?;
}
Rvalue::CopyForDeref(place) => {
let op = &Operand::Copy(*place);
self.validate_operand(op)?
}

Rvalue::Discriminant(place) | Rvalue::Len(place) => {
self.validate_place(place.as_ref())?
Expand Down
11 changes: 10 additions & 1 deletion compiler/rustc_const_eval/src/transform/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
};
}
match rvalue {
Rvalue::Use(_) => {}
Rvalue::Use(_) | Rvalue::CopyForDeref(_) => {}
Rvalue::Aggregate(agg_kind, _) => {
let disallowed = match **agg_kind {
AggregateKind::Array(..) => false,
Expand Down Expand Up @@ -592,6 +592,15 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
),
);
}
if let Rvalue::CopyForDeref(place) = rvalue {
if !place.ty(&self.body.local_decls, self.tcx).ty.builtin_deref(true).is_some()
{
self.fail(
location,
"`CopyForDeref` should only be used for dereferenceable types",
)
}
}
// FIXME(JakobDegen): Check this for all rvalues, not just this one.
if let Rvalue::Use(Operand::Copy(src) | Operand::Move(src)) = rvalue {
// The sides of an assignment must not alias. Currently this just checks whether
Expand Down
14 changes: 13 additions & 1 deletion compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ mod syntax;
pub use syntax::*;
mod switch_sources;
pub mod tcx;
mod terminator;
pub mod terminator;
pub use terminator::*;

pub mod traversal;
Expand Down Expand Up @@ -925,6 +925,15 @@ impl<'tcx> LocalDecl<'tcx> {
}
}

/// Returns `true` if this is a DerefTemp
pub fn is_deref_temp(&self) -> bool {
match self.local_info {
Some(box LocalInfo::DerefTemp) => return true,
_ => (),
}
return false;
}

/// Returns `true` is the local is from a compiler desugaring, e.g.,
/// `__next` from a `for` loop.
#[inline]
Expand Down Expand Up @@ -1795,6 +1804,7 @@ impl<'tcx> Rvalue<'tcx> {
Rvalue::Cast(CastKind::PointerExposeAddress, _, _) => false,

Rvalue::Use(_)
| Rvalue::CopyForDeref(_)
| Rvalue::Repeat(_, _)
| Rvalue::Ref(_, _, _)
| Rvalue::ThreadLocalRef(_)
Expand Down Expand Up @@ -1889,6 +1899,8 @@ impl<'tcx> Debug for Rvalue<'tcx> {
write!(fmt, "&{}{}{:?}", region, kind_str, place)
}

CopyForDeref(ref place) => write!(fmt, "deref_copy {:#?}", place),

AddressOf(mutability, ref place) => {
let kind_str = match mutability {
Mutability::Mut => "mut",
Expand Down
16 changes: 13 additions & 3 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ pub enum MirPhase {
/// of the `mir_promoted` query), these promoted elements are available in the `promoted_mir`
/// query.
ConstsPromoted = 2,
/// After this projections may only contain deref projections as the first element.
Derefered = 3,
/// Beginning with this phase, the following variants are disallowed:
/// * [`TerminatorKind::DropAndReplace`]
/// * [`TerminatorKind::FalseUnwind`]
Expand All @@ -66,9 +68,7 @@ pub enum MirPhase {
/// Furthermore, `Drop` now uses explicit drop flags visible in the MIR and reaching a `Drop`
/// terminator means that the auto-generated drop glue will be invoked. Also, `Copy` operands
/// are allowed for non-`Copy` types.
DropsLowered = 3,
/// After this projections may only contain deref projections as the first element.
Derefered = 4,
DropsLowered = 4,
/// Beginning with this phase, the following variant is disallowed:
/// * [`Rvalue::Aggregate`] for any `AggregateKind` except `Array`
///
Expand Down Expand Up @@ -1051,6 +1051,16 @@ pub enum Rvalue<'tcx> {
/// initialized but its content as uninitialized. Like other pointer casts, this in general
/// affects alias analysis.
ShallowInitBox(Operand<'tcx>, Ty<'tcx>),

/// A CopyForDeref is equivalent to a read from a place at the
/// codegen level, but is treated specially by drop elaboration. When such a read happens, it
/// is guaranteed (via nature of the mir_opt `Derefer` in rustc_mir_transform/src/deref_separator)
/// that the only use of the returned value is a deref operation, immediately
/// followed by one or more projections. Drop elaboration treats this rvalue as if the
/// read never happened and just projects further. This allows simplifying various MIR
/// optimizations and codegen backends that previously had to handle deref operations anywhere
/// in a place.
CopyForDeref(Place<'tcx>),
}

#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ impl<'tcx> Rvalue<'tcx> {
}
},
Rvalue::ShallowInitBox(_, ty) => tcx.mk_box(ty),
Rvalue::CopyForDeref(ref place) => place.ty(local_decls, tcx).ty,
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/mir/type_foldable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ impl<'tcx> TypeFoldable<'tcx> for Rvalue<'tcx> {
Ref(region, bk, place) => {
Ref(region.try_fold_with(folder)?, bk, place.try_fold_with(folder)?)
}
CopyForDeref(place) => CopyForDeref(place.try_fold_with(folder)?),
AddressOf(mutability, place) => AddressOf(mutability, place.try_fold_with(folder)?),
Len(place) => Len(place.try_fold_with(folder)?),
Cast(kind, op, ty) => Cast(kind, op.try_fold_with(folder)?, ty.try_fold_with(folder)?),
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/mir/type_visitable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ impl<'tcx> TypeVisitable<'tcx> for Rvalue<'tcx> {
use crate::mir::Rvalue::*;
match *self {
Use(ref op) => op.visit_with(visitor),
CopyForDeref(ref place) => {
let op = &Operand::Copy(*place);
op.visit_with(visitor)
}
Repeat(ref op, _) => op.visit_with(visitor),
ThreadLocalRef(did) => did.visit_with(visitor),
Ref(region, _, ref place) => {
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,13 @@ macro_rules! make_mir_visitor {
};
self.visit_place(path, ctx, location);
}
Rvalue::CopyForDeref(place) => {
self.visit_place(
place,
PlaceContext::NonMutatingUse(NonMutatingUseContext::Inspect),
location
);
}

Rvalue::AddressOf(m, path) => {
let ctx = match m {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_mir_dataflow/src/impls/borrowed_locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ where
| mir::Rvalue::NullaryOp(..)
| mir::Rvalue::UnaryOp(..)
| mir::Rvalue::Discriminant(..)
| mir::Rvalue::Aggregate(..) => {}
| mir::Rvalue::Aggregate(..)
| mir::Rvalue::CopyForDeref(..) => {}
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_dataflow/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub mod impls;
pub mod move_paths;
pub mod rustc_peek;
pub mod storage;
pub mod un_derefer;

pub(crate) mod indexes {
pub(crate) use super::move_paths::MovePathIndex;
Expand Down
30 changes: 29 additions & 1 deletion compiler/rustc_mir_dataflow/src/move_paths/builder.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::un_derefer::UnDerefer;
use rustc_index::vec::IndexVec;
use rustc_middle::mir::tcx::RvalueInitializationState;
use rustc_middle::mir::*;
Expand All @@ -19,6 +20,7 @@ struct MoveDataBuilder<'a, 'tcx> {
param_env: ty::ParamEnv<'tcx>,
data: MoveData<'tcx>,
errors: Vec<(Place<'tcx>, MoveError<'tcx>)>,
un_derefer: UnDerefer<'tcx>,
}

impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
Expand All @@ -32,6 +34,7 @@ impl<'a, 'tcx> MoveDataBuilder<'a, 'tcx> {
tcx,
param_env,
errors: Vec::new(),
un_derefer: UnDerefer { tcx: tcx, derefer_sidetable: Default::default() },
data: MoveData {
moves: IndexVec::new(),
loc_map: LocationMap::new(body),
Expand Down Expand Up @@ -94,6 +97,11 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
///
/// Maybe we should have separate "borrowck" and "moveck" modes.
fn move_path_for(&mut self, place: Place<'tcx>) -> Result<MovePathIndex, MoveError<'tcx>> {
if let Some(new_place) = self.builder.un_derefer.derefer(place.as_ref(), self.builder.body)
{
return self.move_path_for(new_place);
}

debug!("lookup({:?})", place);
let mut base = self.builder.data.rev_lookup.locals[place.local];

Expand Down Expand Up @@ -276,6 +284,12 @@ struct Gatherer<'b, 'a, 'tcx> {
impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
fn gather_statement(&mut self, stmt: &Statement<'tcx>) {
match &stmt.kind {
StatementKind::Assign(box (place, Rvalue::CopyForDeref(reffed))) => {
assert!(place.projection.is_empty());
if self.builder.body.local_decls[place.local].is_deref_temp() {
self.builder.un_derefer.derefer_sidetable.insert(place.local, *reffed);
}
}
StatementKind::Assign(box (place, rval)) => {
self.create_move_path(*place);
if let RvalueInitializationState::Shallow = rval.initialization_state() {
Expand All @@ -294,7 +308,10 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
}
StatementKind::StorageLive(_) => {}
StatementKind::StorageDead(local) => {
self.gather_move(Place::from(*local));
// DerefTemp locals (results of CopyForDeref) don't actually move anything.
if !self.builder.un_derefer.derefer_sidetable.contains_key(&local) {
self.gather_move(Place::from(*local));
}
}
StatementKind::SetDiscriminant { .. } | StatementKind::Deinit(..) => {
span_bug!(
Expand Down Expand Up @@ -328,6 +345,7 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
self.gather_operand(operand);
}
}
Rvalue::CopyForDeref(..) => unreachable!(),
Rvalue::Ref(..)
| Rvalue::AddressOf(..)
| Rvalue::Discriminant(..)
Expand Down Expand Up @@ -439,6 +457,11 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {

fn gather_move(&mut self, place: Place<'tcx>) {
debug!("gather_move({:?}, {:?})", self.loc, place);
if let Some(new_place) = self.builder.un_derefer.derefer(place.as_ref(), self.builder.body)
{
self.gather_move(new_place);
return;
}

if let [ref base @ .., ProjectionElem::Subslice { from, to, from_end: false }] =
**place.projection
Expand Down Expand Up @@ -494,6 +517,11 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
fn gather_init(&mut self, place: PlaceRef<'tcx>, kind: InitKind) {
debug!("gather_init({:?}, {:?})", self.loc, place);

if let Some(new_place) = self.builder.un_derefer.derefer(place, self.builder.body) {
self.gather_init(new_place.as_ref(), kind);
return;
}

let mut place = place;

// Check if we are assigning into a field of a union, if so, lookup the place
Expand Down
Loading

0 comments on commit 42bd138

Please sign in to comment.