diff --git a/src/librustc_codegen_ssa/mir/analyze.rs b/src/librustc_codegen_ssa/mir/analyze.rs index db935c2b3e265..91d6b3d71c45d 100644 --- a/src/librustc_codegen_ssa/mir/analyze.rs +++ b/src/librustc_codegen_ssa/mir/analyze.rs @@ -53,6 +53,8 @@ struct LocalAnalyzer<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> { // The location of the first visited direct assignment to each // local, or an invalid location (out of bounds `block` index). first_assignment: IndexVec, + + seen_disqualifying_projection: bool, } impl> LocalAnalyzer<'mir, 'a, 'tcx, Bx> { @@ -64,6 +66,7 @@ impl> LocalAnalyzer<'mir, 'a, 'tcx, Bx> { dominators, non_ssa_locals: BitSet::new_empty(fx.mir.local_decls.len()), first_assignment: IndexVec::from_elem(invalid_location, &fx.mir.local_decls), + seen_disqualifying_projection: false, }; // Arguments get assigned to by means of the function being called @@ -95,119 +98,6 @@ impl> LocalAnalyzer<'mir, 'a, 'tcx, Bx> { self.first_assignment[local] = location; } } - - fn process_place( - &mut self, - place_ref: &mir::PlaceRef<'tcx>, - context: PlaceContext, - location: Location, - ) { - let cx = self.fx.cx; - - if let &[ref proj_base @ .., elem] = place_ref.projection { - let mut base_context = if context.is_mutating_use() { - PlaceContext::MutatingUse(MutatingUseContext::Projection) - } else { - PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection) - }; - - // Allow uses of projections that are ZSTs or from scalar fields. - let is_consume = match context { - PlaceContext::NonMutatingUse( - NonMutatingUseContext::Copy | NonMutatingUseContext::Move, - ) => true, - _ => false, - }; - if is_consume { - let base_ty = - mir::Place::ty_from(place_ref.local, proj_base, self.fx.mir, cx.tcx()); - let base_ty = self.fx.monomorphize(&base_ty); - - // ZSTs don't require any actual memory access. - let elem_ty = base_ty.projection_ty(cx.tcx(), elem).ty; - let elem_ty = self.fx.monomorphize(&elem_ty); - let span = self.fx.mir.local_decls[place_ref.local].source_info.span; - if cx.spanned_layout_of(elem_ty, span).is_zst() { - return; - } - - if let mir::ProjectionElem::Field(..) = elem { - let layout = cx.spanned_layout_of(base_ty.ty, span); - if cx.is_backend_immediate(layout) || cx.is_backend_scalar_pair(layout) { - // Recurse with the same context, instead of `Projection`, - // potentially stopping at non-operand projections, - // which would trigger `not_ssa` on locals. - base_context = context; - } - } - } - - if let mir::ProjectionElem::Deref = elem { - // Deref projections typically only read the pointer. - // (the exception being `VarDebugInfo` contexts, handled below) - base_context = PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy); - - // Indirect debuginfo requires going through memory, that only - // the debugger accesses, following our emitted DWARF pointer ops. - // - // FIXME(eddyb) Investigate the possibility of relaxing this, but - // note that `llvm.dbg.declare` *must* be used for indirect places, - // even if we start using `llvm.dbg.value` for all other cases, - // as we don't necessarily know when the value changes, but only - // where it lives in memory. - // - // It's possible `llvm.dbg.declare` could support starting from - // a pointer that doesn't point to an `alloca`, but this would - // only be useful if we know the pointer being `Deref`'d comes - // from an immutable place, and if `llvm.dbg.declare` calls - // must be at the very start of the function, then only function - // arguments could contain such pointers. - if context == PlaceContext::NonUse(NonUseContext::VarDebugInfo) { - // We use `NonUseContext::VarDebugInfo` for the base, - // which might not force the base local to memory, - // so we have to do it manually. - self.visit_local(&place_ref.local, context, location); - } - } - - // `NonUseContext::VarDebugInfo` needs to flow all the - // way down to the base local (see `visit_local`). - if context == PlaceContext::NonUse(NonUseContext::VarDebugInfo) { - base_context = context; - } - - self.process_place( - &mir::PlaceRef { local: place_ref.local, projection: proj_base }, - base_context, - location, - ); - // HACK(eddyb) this emulates the old `visit_projection_elem`, this - // entire `visit_place`-like `process_place` method should be rewritten, - // now that we have moved to the "slice of projections" representation. - if let mir::ProjectionElem::Index(local) = elem { - self.visit_local( - &local, - PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy), - location, - ); - } - } else { - // FIXME this is super_place code, is repeated here to avoid cloning place or changing - // visit_place API - let mut context = context; - - if !place_ref.projection.is_empty() { - context = if context.is_mutating_use() { - PlaceContext::MutatingUse(MutatingUseContext::Projection) - } else { - PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection) - }; - } - - self.visit_local(&place_ref.local, context, location); - self.visit_projection(place_ref.local, place_ref.projection, context, location); - } - } } impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> @@ -264,10 +154,23 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> fn visit_place(&mut self, place: &mir::Place<'tcx>, context: PlaceContext, location: Location) { debug!("visit_place(place={:?}, context={:?})", place, context); - self.process_place(&place.as_ref(), context, location); + + assert!(!self.seen_disqualifying_projection); + self.super_place(place, context, location); + self.seen_disqualifying_projection = false; } - fn visit_local(&mut self, &local: &mir::Local, context: PlaceContext, location: Location) { + fn visit_local( + &mut self, + &local: &mir::Local, + context: PlaceContext, + _: bool, + location: Location, + ) { + if self.seen_disqualifying_projection { + self.not_ssa(local); + } + match context { PlaceContext::MutatingUse(MutatingUseContext::Call) | PlaceContext::MutatingUse(MutatingUseContext::Yield) => { @@ -276,8 +179,11 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> PlaceContext::NonUse(_) | PlaceContext::MutatingUse(MutatingUseContext::Retag) => {} - PlaceContext::NonMutatingUse( - NonMutatingUseContext::Copy | NonMutatingUseContext::Move, + PlaceContext::MutatingUse(MutatingUseContext::Deref) + | PlaceContext::NonMutatingUse( + NonMutatingUseContext::Copy + | NonMutatingUseContext::Move + | NonMutatingUseContext::Deref, ) => { // Reads from uninitialized variables (e.g., in dead code, after // optimizations) require locals to be in (uninitialized) memory. @@ -298,16 +204,14 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> MutatingUseContext::Store | MutatingUseContext::AsmOutput | MutatingUseContext::Borrow - | MutatingUseContext::AddressOf - | MutatingUseContext::Projection, + | MutatingUseContext::AddressOf, ) | PlaceContext::NonMutatingUse( NonMutatingUseContext::Inspect | NonMutatingUseContext::SharedBorrow | NonMutatingUseContext::UniqueBorrow | NonMutatingUseContext::ShallowBorrow - | NonMutatingUseContext::AddressOf - | NonMutatingUseContext::Projection, + | NonMutatingUseContext::AddressOf, ) => { self.not_ssa(local); } @@ -323,6 +227,87 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> } } } + + fn visit_projection_elem( + &mut self, + local: mir::Local, + proj_base: &[mir::PlaceElem<'tcx>], + elem: mir::PlaceElem<'tcx>, + context: PlaceContext, + location: Location, + ) { + // `super_projection_elem` can call `visit_local` without first calling `visit_place`. + let old = std::mem::replace(&mut self.seen_disqualifying_projection, false); + self.super_projection_elem(local, proj_base, elem, context, location); + self.seen_disqualifying_projection = old; + + // Arbitrary projections on pointer dereferences (e.g. `(*x)[12]`) do not require that + // `x` live on the stack. On the other hand, `*(x[12])` does necessitate that `x` go on + // the stack. + if let mir::ProjectionElem::Deref = elem { + // Indirect debuginfo requires going through memory, that only + // the debugger accesses, following our emitted DWARF pointer ops. + // + // FIXME(eddyb) Investigate the possibility of relaxing this, but + // note that `llvm.dbg.declare` *must* be used for indirect places, + // even if we start using `llvm.dbg.value` for all other cases, + // as we don't necessarily know when the value changes, but only + // where it lives in memory. + // + // It's possible `llvm.dbg.declare` could support starting from + // a pointer that doesn't point to an `alloca`, but this would + // only be useful if we know the pointer being `Deref`'d comes + // from an immutable place, and if `llvm.dbg.declare` calls + // must be at the very start of the function, then only function + // arguments could contain such pointers. + if context == PlaceContext::NonUse(NonUseContext::VarDebugInfo) { + self.not_ssa(local); + } + + self.seen_disqualifying_projection = false; + return; + } + + // For anything besides a simple read, all projections result in the base local being + // put onto the stack. + let is_consume = matches!( + context, + PlaceContext::NonMutatingUse( + NonMutatingUseContext::Copy + | NonMutatingUseContext::Move + | NonMutatingUseContext::Deref + ) | PlaceContext::MutatingUse(MutatingUseContext::Deref) + ); + if !is_consume { + self.seen_disqualifying_projection = true; + return; + } + + let cx = self.fx.cx; + + let base_ty = mir::Place::ty_from(local, proj_base, self.fx.mir, cx.tcx()); + let base_ty = self.fx.monomorphize(&base_ty); + + // ZSTs don't require any actual memory access. + let elem_ty = base_ty.projection_ty(cx.tcx(), elem).ty; + let elem_ty = self.fx.monomorphize(&elem_ty); + let span = self.fx.mir.local_decls[local].source_info.span; + if cx.spanned_layout_of(elem_ty, span).is_zst() { + return; + } + + // Don't force locals on the stack if one of their fields is read that is eligible + // for an immediate. + if let mir::ProjectionElem::Field(..) = elem { + let layout = cx.spanned_layout_of(base_ty.ty, span); + if cx.is_backend_immediate(layout) || cx.is_backend_scalar_pair(layout) { + return; + } + } + + // Anything besides a field or ZST projection requires that the base go on the stack + self.seen_disqualifying_projection = true; + } } #[derive(Copy, Clone, Debug, PartialEq, Eq)] diff --git a/src/librustc_middle/mir/visit.rs b/src/librustc_middle/mir/visit.rs index 2efc5f1dabedc..b44eb581e8432 100644 --- a/src/librustc_middle/mir/visit.rs +++ b/src/librustc_middle/mir/visit.rs @@ -216,10 +216,13 @@ macro_rules! make_mir_visitor { self.super_var_debug_info(var_debug_info); } - fn visit_local(&mut self, - _local: & $($mutability)? Local, - _context: PlaceContext, - _location: Location) { + fn visit_local( + &mut self, + _local: & $($mutability)? Local, + _context: PlaceContext, + _has_projections: bool, + _location: Location + ) { } fn visit_source_scope(&mut self, @@ -357,6 +360,7 @@ macro_rules! make_mir_visitor { self.visit_local( local, PlaceContext::NonUse(NonUseContext::StorageLive), + false, location ); } @@ -364,6 +368,7 @@ macro_rules! make_mir_visitor { self.visit_local( local, PlaceContext::NonUse(NonUseContext::StorageDead), + false, location ); } @@ -428,6 +433,7 @@ macro_rules! make_mir_visitor { self.visit_local( & $($mutability)? local, PlaceContext::NonMutatingUse(NonMutatingUseContext::Move), + false, location, ); @@ -880,11 +886,11 @@ macro_rules! visit_place_fns { context: PlaceContext, location: Location, ) { - self.visit_local(&mut place.local, context, location); - if let Some(new_projection) = self.process_projection(&place.projection, location) { place.projection = self.tcx().intern_place_elems(&new_projection); } + + self.visit_local(&mut place.local, context, !place.projection.is_empty(), location); } fn process_projection( @@ -922,6 +928,7 @@ macro_rules! visit_place_fns { self.visit_local( &mut new_local, PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy), + false, location, ); @@ -937,16 +944,6 @@ macro_rules! visit_place_fns { }; () => { - fn visit_projection( - &mut self, - local: Local, - projection: &[PlaceElem<'tcx>], - context: PlaceContext, - location: Location, - ) { - self.super_projection(local, projection, context, location); - } - fn visit_projection_elem( &mut self, local: Local, @@ -958,33 +955,57 @@ macro_rules! visit_place_fns { self.super_projection_elem(local, proj_base, elem, context, location); } - fn super_place(&mut self, place: &Place<'tcx>, context: PlaceContext, location: Location) { - let mut context = context; - - if !place.projection.is_empty() { - context = if context.is_mutating_use() { - PlaceContext::MutatingUse(MutatingUseContext::Projection) - } else { - PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection) - }; - } - - self.visit_local(&place.local, context, location); - - self.visit_projection(place.local, &place.projection, context, location); + /// Visits each `ProjectionElem`, starting with the outermost one and working in, along + /// with a slice of all the projections we have yet to visit. + /// + /// For `(*(x.y)).z`, this gives us, in order: + /// - outermost=`Field(z)` base=`[Field(y), Deref]` context=`Copy` + /// - outermost=`Deref` base=`[Field(y)]` context=`Copy` + /// - outermost=`Field(y)` base=`[]` context=`Deref` + /// + /// Finally, `x` will be visited by `visit_local`. + /// + /// Context is preserved as we move inwards unless we see a `Deref` projection, so both + /// `x.y = 4` and `x = 4` are treated as a `MutatingPlaceContext::Store`. `Deref` + /// projections are handled specially since they change what memory is referred to by a + /// `Place`. For example, `*x = 4` is very different from `x = 4` when we want to know the + /// possible definitions of `x`. + fn super_place( + &mut self, + place: &Place<'tcx>, + mut context: PlaceContext, + location: Location, + ) { + self.super_projection(place.local, place.projection, &mut context, location); + self.visit_local(&place.local, context, !place.projection.is_empty(), location); } fn super_projection( &mut self, local: Local, - projection: &[PlaceElem<'tcx>], - context: PlaceContext, + mut projection: &[PlaceElem<'tcx>], + context: &mut PlaceContext, location: Location, ) { - let mut cursor = projection; - while let &[ref proj_base @ .., elem] = cursor { - cursor = proj_base; - self.visit_projection_elem(local, cursor, elem, context, location); + while let [base @ .., outermost] = projection { + self.visit_projection_elem(local, base, *outermost, *context, location); + projection = base; + + match outermost { + ProjectionElem::Deref => { + if context.is_mutating_use() { + *context = PlaceContext::MutatingUse(MutatingUseContext::Deref); + } else { + *context = PlaceContext::NonMutatingUse(NonMutatingUseContext::Deref); + }; + } + + ProjectionElem::Field(..) + | ProjectionElem::Index(_) + | ProjectionElem::Subslice { .. } + | ProjectionElem::ConstantIndex { .. } + | ProjectionElem::Downcast(..) => {} + } } } @@ -1004,6 +1025,7 @@ macro_rules! visit_place_fns { self.visit_local( &local, PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy), + false, location, ); } @@ -1081,13 +1103,8 @@ pub enum NonMutatingUseContext { UniqueBorrow, /// AddressOf for *const pointer. AddressOf, - /// Used as base for another place, e.g., `x` in `x.y`. Will not mutate the place. - /// For example, the projection `x.y` is not marked as a mutation in these cases: - /// - /// z = x.y; - /// f(&x.y); - /// - Projection, + /// A dereference, e.g., `*x` or `*x.f[0]`. + Deref, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -1108,13 +1125,8 @@ pub enum MutatingUseContext { Borrow, /// AddressOf for *mut pointer. AddressOf, - /// Used as base for another place, e.g., `x` in `x.y`. Could potentially mutate the place. - /// For example, the projection `x.y` is marked as a mutation in these cases: - /// - /// x.y = ...; - /// f(&mut x.y); - /// - Projection, + /// A dereference, e.g., `*x` or `*x.f[0]`. + Deref, /// Retagging, a "Stacked Borrows" shadow state operation Retag, } diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index ef9af7bace96f..072f3d0f58e16 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -96,7 +96,7 @@ impl LocalsStateAtExit { struct HasStorageDead(BitSet); impl<'tcx> Visitor<'tcx> for HasStorageDead { - fn visit_local(&mut self, local: &Local, ctx: PlaceContext, _: Location) { + fn visit_local(&mut self, local: &Local, ctx: PlaceContext, _: bool, _: Location) { if ctx == PlaceContext::NonUse(NonUseContext::StorageDead) { self.0.insert(*local); } @@ -214,7 +214,13 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'tcx> { self.super_assign(assigned_place, rvalue, location) } - fn visit_local(&mut self, temp: &Local, context: PlaceContext, location: Location) { + fn visit_local( + &mut self, + temp: &Local, + context: PlaceContext, + has_projections: bool, + location: Location, + ) { if !context.is_use() { return; } @@ -230,6 +236,7 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'tcx> { // doesn't count as an activation. =) if borrow_data.reserve_location == location && context == PlaceContext::MutatingUse(MutatingUseContext::Store) + && !has_projections { return; } diff --git a/src/librustc_mir/borrow_check/def_use.rs b/src/librustc_mir/borrow_check/def_use.rs index 689ec249a2fb4..5fa83e633e264 100644 --- a/src/librustc_mir/borrow_check/def_use.rs +++ b/src/librustc_mir/borrow_check/def_use.rs @@ -40,8 +40,8 @@ pub fn categorize(context: PlaceContext) -> Option { // purposes of NLL, these are special in that **all** the // lifetimes appearing in the variable must be live for each regular use. - PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection) | - PlaceContext::MutatingUse(MutatingUseContext::Projection) | + PlaceContext::NonMutatingUse(NonMutatingUseContext::Deref) | + PlaceContext::MutatingUse(MutatingUseContext::Deref) | // Borrows only consider their local used at the point of the borrow. // This won't affect the results since we use this analysis for generators diff --git a/src/librustc_mir/borrow_check/diagnostics/find_use.rs b/src/librustc_mir/borrow_check/diagnostics/find_use.rs index 8d8cdfb52934c..9683391fb85cc 100644 --- a/src/librustc_mir/borrow_check/diagnostics/find_use.rs +++ b/src/librustc_mir/borrow_check/diagnostics/find_use.rs @@ -106,7 +106,13 @@ enum DefUseResult { } impl<'cx, 'tcx> Visitor<'tcx> for DefUseVisitor<'cx, 'tcx> { - fn visit_local(&mut self, &local: &Local, context: PlaceContext, _: Location) { + fn visit_local( + &mut self, + &local: &Local, + context: PlaceContext, + has_projections: bool, + _: Location, + ) { let local_ty = self.body.local_decls[local].ty; let mut found_it = false; @@ -118,6 +124,10 @@ impl<'cx, 'tcx> Visitor<'tcx> for DefUseVisitor<'cx, 'tcx> { if found_it { self.def_use_result = match def_use::categorize(context) { + // FIXME: This is to preserve the old behavior when `PlaceContext::Projection` was + // a thing. + _ if has_projections => Some(DefUseResult::UseLive { local }), + Some(DefUse::Def) => Some(DefUseResult::Def), Some(DefUse::Use) => Some(DefUseResult::UseLive { local }), Some(DefUse::Drop) => Some(DefUseResult::UseDrop { local }), diff --git a/src/librustc_mir/borrow_check/type_check/liveness/local_use_map.rs b/src/librustc_mir/borrow_check/type_check/liveness/local_use_map.rs index 995e3a60a0c76..ba8de80c4790b 100644 --- a/src/librustc_mir/borrow_check/type_check/liveness/local_use_map.rs +++ b/src/librustc_mir/borrow_check/type_check/liveness/local_use_map.rs @@ -157,9 +157,19 @@ impl LocalUseMapBuild<'_> { } impl Visitor<'tcx> for LocalUseMapBuild<'_> { - fn visit_local(&mut self, &local: &Local, context: PlaceContext, location: Location) { + fn visit_local( + &mut self, + &local: &Local, + context: PlaceContext, + has_projections: bool, + location: Location, + ) { if self.locals_with_use_data[local] { match def_use::categorize(context) { + // FIXME: This is to preserve the old behavior when `PlaceContext::Projection` was + // a thing. + _ if has_projections => self.insert_use(local, location), + Some(DefUse::Def) => self.insert_def(local, location), Some(DefUse::Use) => self.insert_use(local, location), Some(DefUse::Drop) => self.insert_drop(local, location), diff --git a/src/librustc_mir/borrow_check/type_check/liveness/polonius.rs b/src/librustc_mir/borrow_check/type_check/liveness/polonius.rs index d285098c52ad2..48760f26ac3b1 100644 --- a/src/librustc_mir/borrow_check/type_check/liveness/polonius.rs +++ b/src/librustc_mir/borrow_check/type_check/liveness/polonius.rs @@ -55,8 +55,18 @@ impl UseFactsExtractor<'_> { } impl Visitor<'tcx> for UseFactsExtractor<'_> { - fn visit_local(&mut self, &local: &Local, context: PlaceContext, location: Location) { + fn visit_local( + &mut self, + &local: &Local, + context: PlaceContext, + has_projections: bool, + location: Location, + ) { match def_use::categorize(context) { + // FIXME: This is to preserve the old behavior when `PlaceContext::Projection` was + // a thing. + _ if has_projections => self.insert_use(local, location), + Some(DefUse::Def) => self.insert_def(local, location), Some(DefUse::Use) => self.insert_use(local, location), Some(DefUse::Drop) => self.insert_drop_use(local, location), diff --git a/src/librustc_mir/borrow_check/used_muts.rs b/src/librustc_mir/borrow_check/used_muts.rs index e027056842db9..42bc0c569a25a 100644 --- a/src/librustc_mir/borrow_check/used_muts.rs +++ b/src/librustc_mir/borrow_check/used_muts.rs @@ -92,8 +92,17 @@ impl<'visit, 'cx, 'tcx> Visitor<'tcx> for GatherUsedMutsVisitor<'visit, 'cx, 'tc self.super_statement(statement, location); } - fn visit_local(&mut self, local: &Local, place_context: PlaceContext, location: Location) { - if place_context.is_place_assignment() && self.temporary_used_locals.contains(local) { + fn visit_local( + &mut self, + local: &Local, + place_context: PlaceContext, + has_projections: bool, + location: Location, + ) { + if place_context.is_place_assignment() + && self.temporary_used_locals.contains(local) + && !has_projections + { // Propagate the Local assigned at this Location as a used mutable local variable for moi in &self.mbcx.move_data.loc_map[location] { let mpi = &self.mbcx.move_data.moves[*moi].path; diff --git a/src/librustc_mir/dataflow/impls/init_locals.rs b/src/librustc_mir/dataflow/impls/init_locals.rs index 01cb794a2e085..3dc16b7c635be 100644 --- a/src/librustc_mir/dataflow/impls/init_locals.rs +++ b/src/librustc_mir/dataflow/impls/init_locals.rs @@ -81,19 +81,32 @@ impl Visitor<'tcx> for TransferFunction<'a, T> where T: GenKill, { - fn visit_local(&mut self, &local: &Local, context: PlaceContext, _: Location) { + fn visit_local( + &mut self, + &local: &Local, + context: PlaceContext, + has_projections: bool, + _: Location, + ) { use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, NonUseContext}; match context { // These are handled specially in `call_return_effect` and `yield_resume_effect`. PlaceContext::MutatingUse(MutatingUseContext::Call | MutatingUseContext::Yield) => {} + // `*x = 4` does not mutate `x`. Treat it the same as a use. + PlaceContext::MutatingUse(MutatingUseContext::Deref) => {} + // Otherwise, when a place is mutated, we must consider it possibly initialized. PlaceContext::MutatingUse(_) => self.trans.gen(local), - // If the local is moved out of, or if it gets marked `StorageDead`, consider it no - // longer initialized. + // If the local is moved out of entirely, or if it gets marked `StorageDead`, consider + // it no longer initialized. PlaceContext::NonUse(NonUseContext::StorageDead) - | PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) => self.trans.kill(local), + | PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) => { + if !has_projections { + self.trans.kill(local); + } + } // All other uses do not affect this analysis. PlaceContext::NonUse( @@ -108,7 +121,7 @@ where | NonMutatingUseContext::ShallowBorrow | NonMutatingUseContext::UniqueBorrow | NonMutatingUseContext::AddressOf - | NonMutatingUseContext::Projection, + | NonMutatingUseContext::Deref, ) => {} } } diff --git a/src/librustc_mir/dataflow/impls/liveness.rs b/src/librustc_mir/dataflow/impls/liveness.rs index d24faacd3779e..74aa17170f228 100644 --- a/src/librustc_mir/dataflow/impls/liveness.rs +++ b/src/librustc_mir/dataflow/impls/liveness.rs @@ -92,9 +92,15 @@ impl<'tcx, T> Visitor<'tcx> for TransferFunction<'_, T> where T: GenKill, { - fn visit_local(&mut self, &local: &Local, context: PlaceContext, _: Location) { + fn visit_local( + &mut self, + &local: &Local, + context: PlaceContext, + has_projections: bool, + _: Location, + ) { match DefUse::for_place(context) { - Some(DefUse::Def) => self.0.kill(local), + Some(DefUse::Def) if !has_projections => self.0.kill(local), Some(DefUse::Use) => self.0.gen(local), _ => {} } @@ -126,7 +132,7 @@ impl DefUse { | MutatingUseContext::AsmOutput | MutatingUseContext::Borrow | MutatingUseContext::Drop - | MutatingUseContext::Projection + | MutatingUseContext::Deref | MutatingUseContext::Retag, ) | PlaceContext::NonMutatingUse( @@ -134,7 +140,7 @@ impl DefUse { | NonMutatingUseContext::Copy | NonMutatingUseContext::Inspect | NonMutatingUseContext::Move - | NonMutatingUseContext::Projection + | NonMutatingUseContext::Deref | NonMutatingUseContext::ShallowBorrow | NonMutatingUseContext::SharedBorrow | NonMutatingUseContext::UniqueBorrow, diff --git a/src/librustc_mir/dataflow/impls/storage_liveness.rs b/src/librustc_mir/dataflow/impls/storage_liveness.rs index cd04493c092e0..5036ffd156755 100644 --- a/src/librustc_mir/dataflow/impls/storage_liveness.rs +++ b/src/librustc_mir/dataflow/impls/storage_liveness.rs @@ -298,8 +298,15 @@ impl<'a, 'mir, 'tcx, T> Visitor<'tcx> for MoveVisitor<'a, 'mir, 'tcx, T> where T: GenKill, { - fn visit_local(&mut self, local: &Local, context: PlaceContext, loc: Location) { - if PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) == context { + fn visit_local( + &mut self, + local: &Local, + context: PlaceContext, + has_projections: bool, + loc: Location, + ) { + if PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) == context && !has_projections + { let mut borrowed_locals = self.borrowed_locals.borrow_mut(); borrowed_locals.seek_before_primary_effect(loc); if !borrowed_locals.contains(*local) { diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index 36f3947d83017..756064398ac4a 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -678,6 +678,7 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> { &mut self, _place_local: &Local, _context: mir::visit::PlaceContext, + _has_projections: bool, _location: Location, ) { } diff --git a/src/librustc_mir/transform/check_consts/validation.rs b/src/librustc_mir/transform/check_consts/validation.rs index 35a8df62cb83a..af7c0b946b4cd 100644 --- a/src/librustc_mir/transform/check_consts/validation.rs +++ b/src/librustc_mir/transform/check_consts/validation.rs @@ -4,7 +4,7 @@ use rustc_errors::struct_span_err; use rustc_hir::{self as hir, lang_items}; use rustc_hir::{def_id::DefId, HirId}; use rustc_infer::infer::TyCtxtInferExt; -use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor}; +use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor}; use rustc_middle::mir::*; use rustc_middle::ty::cast::CastTy; use rustc_middle::ty::{self, Instance, InstanceDef, TyCtxt}; @@ -272,38 +272,32 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> { trace!("visit_rvalue: rvalue={:?} location={:?}", rvalue, location); // Special-case reborrows to be more like a copy of a reference. + // + // In other words, `&*(x.f)` is treated like a copy of `x.f`. match *rvalue { Rvalue::Ref(_, kind, place) => { if let Some(reborrowed_proj) = place_as_reborrow(self.tcx, self.body, place) { - let ctx = match kind { - BorrowKind::Shared => { - PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow) - } - BorrowKind::Shallow => { - PlaceContext::NonMutatingUse(NonMutatingUseContext::ShallowBorrow) - } - BorrowKind::Unique => { - PlaceContext::NonMutatingUse(NonMutatingUseContext::UniqueBorrow) + let mut ctx = match kind { + BorrowKind::Shared | BorrowKind::Shallow | BorrowKind::Unique => { + PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) } BorrowKind::Mut { .. } => { - PlaceContext::MutatingUse(MutatingUseContext::Borrow) + // `&mut` does not implement `Copy`. While `Copy` vs. `Move` shouldn't + // matter in this context, use the correct `PlaceContext` regardless. + PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) } }; - self.visit_local(&place.local, ctx, location); - self.visit_projection(place.local, reborrowed_proj, ctx, location); + + self.visit_local(&place.local, ctx, !reborrowed_proj.is_empty(), location); + self.super_projection(place.local, reborrowed_proj, &mut ctx, location); return; } } - Rvalue::AddressOf(mutbl, place) => { + Rvalue::AddressOf(_, place) => { if let Some(reborrowed_proj) = place_as_reborrow(self.tcx, self.body, place) { - let ctx = match mutbl { - Mutability::Not => { - PlaceContext::NonMutatingUse(NonMutatingUseContext::AddressOf) - } - Mutability::Mut => PlaceContext::MutatingUse(MutatingUseContext::AddressOf), - }; - self.visit_local(&place.local, ctx, location); - self.visit_projection(place.local, reborrowed_proj, ctx, location); + let mut ctx = PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy); + self.visit_local(&place.local, ctx, !reborrowed_proj.is_empty(), location); + self.super_projection(place.local, reborrowed_proj, &mut ctx, location); return; } } diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 529e63ab96791..e0dcbefb94f9e 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -763,15 +763,12 @@ impl CanConstProp { } impl<'tcx> Visitor<'tcx> for CanConstProp { - fn visit_local(&mut self, &local: &Local, context: PlaceContext, _: Location) { + fn visit_local(&mut self, &local: &Local, context: PlaceContext, _: bool, _: Location) { use rustc_middle::mir::visit::PlaceContext::*; match context { - // Projections are fine, because `&mut foo.x` will be caught by - // `MutatingUseContext::Borrow` elsewhere. - MutatingUse(MutatingUseContext::Projection) // These are just stores, where the storing is not propagatable, but there may be later // mutations of the same local via `Store` - | MutatingUse(MutatingUseContext::Call) + MutatingUse(MutatingUseContext::Call) // Actual store that can possibly even propagate a value | MutatingUse(MutatingUseContext::Store) => { if !self.found_assignment.insert(local) { @@ -795,7 +792,7 @@ impl<'tcx> Visitor<'tcx> for CanConstProp { NonMutatingUse(NonMutatingUseContext::Copy) | NonMutatingUse(NonMutatingUseContext::Move) | NonMutatingUse(NonMutatingUseContext::Inspect) - | NonMutatingUse(NonMutatingUseContext::Projection) + | NonMutatingUse(NonMutatingUseContext::Deref) | NonUse(_) => {} // These could be propagated with a smarter analysis or just some careful thinking about @@ -803,6 +800,7 @@ impl<'tcx> Visitor<'tcx> for CanConstProp { MutatingUse(MutatingUseContext::AsmOutput) | MutatingUse(MutatingUseContext::Yield) | MutatingUse(MutatingUseContext::Drop) + | MutatingUse(MutatingUseContext::Deref) | MutatingUse(MutatingUseContext::Retag) // These can't ever be propagated under any scheme, as we can't reason about indirect // mutation. diff --git a/src/librustc_mir/transform/generator.rs b/src/librustc_mir/transform/generator.rs index c8702eeae1d5b..ccc2f0ac9ec9a 100644 --- a/src/librustc_mir/transform/generator.rs +++ b/src/librustc_mir/transform/generator.rs @@ -87,7 +87,7 @@ impl<'tcx> MutVisitor<'tcx> for RenameLocalVisitor<'tcx> { self.tcx } - fn visit_local(&mut self, local: &mut Local, _: PlaceContext, _: Location) { + fn visit_local(&mut self, local: &mut Local, _: PlaceContext, _: bool, _: Location) { if *local == self.from { *local = self.to; } @@ -113,7 +113,7 @@ impl<'tcx> MutVisitor<'tcx> for DerefArgVisitor<'tcx> { self.tcx } - fn visit_local(&mut self, local: &mut Local, _: PlaceContext, _: Location) { + fn visit_local(&mut self, local: &mut Local, _: PlaceContext, _: bool, _: Location) { assert_ne!(*local, SELF_ARG); } @@ -128,7 +128,7 @@ impl<'tcx> MutVisitor<'tcx> for DerefArgVisitor<'tcx> { self.tcx, ); } else { - self.visit_local(&mut place.local, context, location); + self.visit_local(&mut place.local, context, !place.projection.is_empty(), location); for elem in place.projection.iter() { if let PlaceElem::Index(local) = elem { @@ -149,7 +149,7 @@ impl<'tcx> MutVisitor<'tcx> for PinArgVisitor<'tcx> { self.tcx } - fn visit_local(&mut self, local: &mut Local, _: PlaceContext, _: Location) { + fn visit_local(&mut self, local: &mut Local, _: PlaceContext, _: bool, _: Location) { assert_ne!(*local, SELF_ARG); } @@ -167,7 +167,7 @@ impl<'tcx> MutVisitor<'tcx> for PinArgVisitor<'tcx> { self.tcx, ); } else { - self.visit_local(&mut place.local, context, location); + self.visit_local(&mut place.local, context, !place.projection.is_empty(), location); for elem in place.projection.iter() { if let PlaceElem::Index(local) = elem { @@ -284,7 +284,7 @@ impl MutVisitor<'tcx> for TransformVisitor<'tcx> { self.tcx } - fn visit_local(&mut self, local: &mut Local, _: PlaceContext, _: Location) { + fn visit_local(&mut self, local: &mut Local, _: PlaceContext, _: bool, _: Location) { assert_eq!(self.remap.get(local), None); } diff --git a/src/librustc_mir/transform/inline.rs b/src/librustc_mir/transform/inline.rs index 068d055fa78f8..0a66ae3f3d8a9 100644 --- a/src/librustc_mir/transform/inline.rs +++ b/src/librustc_mir/transform/inline.rs @@ -679,7 +679,13 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Integrator<'a, 'tcx> { self.tcx } - fn visit_local(&mut self, local: &mut Local, _ctxt: PlaceContext, _location: Location) { + fn visit_local( + &mut self, + local: &mut Local, + _ctxt: PlaceContext, + _: bool, + _location: Location, + ) { *local = self.make_integrate_local(*local); } diff --git a/src/librustc_mir/transform/nrvo.rs b/src/librustc_mir/transform/nrvo.rs index 1f3d7bb7cc6f4..d5096a52a0c68 100644 --- a/src/librustc_mir/transform/nrvo.rs +++ b/src/librustc_mir/transform/nrvo.rs @@ -196,7 +196,7 @@ impl MutVisitor<'tcx> for RenameToReturnPlace<'tcx> { self.super_terminator(terminator, loc); } - fn visit_local(&mut self, l: &mut Local, _: PlaceContext, _: Location) { + fn visit_local(&mut self, l: &mut Local, _: PlaceContext, _: bool, _: Location) { assert_ne!(*l, mir::RETURN_PLACE); if *l == self.to_rename { *l = mir::RETURN_PLACE; @@ -215,7 +215,7 @@ impl IsReturnPlaceRead { } impl Visitor<'tcx> for IsReturnPlaceRead { - fn visit_local(&mut self, &l: &Local, ctxt: PlaceContext, _: Location) { + fn visit_local(&mut self, &l: &Local, ctxt: PlaceContext, _: bool, _: Location) { if l == mir::RETURN_PLACE && ctxt.is_use() && !ctxt.is_place_assignment() { self.0 = true; } diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs index 330f6c1640ff4..37a09c0894b77 100644 --- a/src/librustc_mir/transform/promote_consts.rs +++ b/src/librustc_mir/transform/promote_consts.rs @@ -150,7 +150,13 @@ struct Collector<'a, 'tcx> { } impl<'tcx> Visitor<'tcx> for Collector<'_, 'tcx> { - fn visit_local(&mut self, &index: &Local, context: PlaceContext, location: Location) { + fn visit_local( + &mut self, + &index: &Local, + context: PlaceContext, + has_projections: bool, + location: Location, + ) { debug!("visit_local: index={:?} context={:?} location={:?}", index, context, location); // We're only interested in temporaries and the return place match self.ccx.body.local_kind(index) { @@ -161,7 +167,7 @@ impl<'tcx> Visitor<'tcx> for Collector<'_, 'tcx> { // Ignore drops, if the temp gets promoted, // then it's constant and thus drop is noop. // Non-uses are also irrelevant. - if context.is_drop() || !context.is_use() { + if context.is_drop() && !has_projections || !context.is_use() { debug!( "visit_local: context.is_drop={:?} context.is_use={:?}", context.is_drop(), @@ -175,18 +181,20 @@ impl<'tcx> Visitor<'tcx> for Collector<'_, 'tcx> { if *temp == TempState::Undefined { match context { PlaceContext::MutatingUse(MutatingUseContext::Store) - | PlaceContext::MutatingUse(MutatingUseContext::Call) => { + | PlaceContext::MutatingUse(MutatingUseContext::Call) + if !has_projections => + { *temp = TempState::Defined { location, uses: 0 }; return; } _ => { /* mark as unpromotable below */ } } } else if let TempState::Defined { ref mut uses, .. } = *temp { - // We always allow borrows, even mutable ones, as we need - // to promote mutable borrows of some ZSTs e.g., `&mut []`. let allowed_use = match context { - PlaceContext::MutatingUse(MutatingUseContext::Borrow) - | PlaceContext::NonMutatingUse(_) => true, + // We allow borrows of unprojected locals, even mutable ones, as we need + // to promote mutable borrows of some ZSTs e.g., `&mut []`. + PlaceContext::MutatingUse(MutatingUseContext::Borrow) if !has_projections => true, + PlaceContext::NonMutatingUse(_) => true, PlaceContext::MutatingUse(_) | PlaceContext::NonUse(_) => false, }; debug!("visit_local: allowed_use={:?}", allowed_use); @@ -1091,7 +1099,7 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Promoter<'a, 'tcx> { self.tcx } - fn visit_local(&mut self, local: &mut Local, _: PlaceContext, _: Location) { + fn visit_local(&mut self, local: &mut Local, _: PlaceContext, _: bool, _: Location) { if self.is_temp_kind(*local) { *local = self.promote_temp(*local); } diff --git a/src/librustc_mir/transform/simplify.rs b/src/librustc_mir/transform/simplify.rs index 491c37cbe06e8..8ba45bf01c0bb 100644 --- a/src/librustc_mir/transform/simplify.rs +++ b/src/librustc_mir/transform/simplify.rs @@ -382,7 +382,13 @@ impl<'a, 'tcx> DeclMarker<'a, 'tcx> { } impl<'a, 'tcx> Visitor<'tcx> for DeclMarker<'a, 'tcx> { - fn visit_local(&mut self, local: &Local, ctx: PlaceContext, location: Location) { + fn visit_local( + &mut self, + local: &Local, + ctx: PlaceContext, + has_projections: bool, + location: Location, + ) { // Ignore storage markers altogether, they get removed along with their otherwise unused // decls. // FIXME: Extend this to all non-uses. @@ -393,9 +399,7 @@ impl<'a, 'tcx> Visitor<'tcx> for DeclMarker<'a, 'tcx> { // Ignore stores of constants because `ConstProp` and `CopyProp` can remove uses of many // of these locals. However, if the local is still needed, then it will be referenced in // another place and we'll mark it as being used there. - if ctx == PlaceContext::MutatingUse(MutatingUseContext::Store) - || ctx == PlaceContext::MutatingUse(MutatingUseContext::Projection) - { + if ctx == PlaceContext::MutatingUse(MutatingUseContext::Store) || has_projections { let block = &self.body.basic_blocks()[location.block]; if location.statement_index != block.statements.len() { let stmt = &block.statements[location.statement_index]; @@ -444,10 +448,16 @@ impl<'a, 'tcx> StatementDeclMarker<'a, 'tcx> { } impl<'a, 'tcx> Visitor<'tcx> for StatementDeclMarker<'a, 'tcx> { - fn visit_local(&mut self, local: &Local, context: PlaceContext, _location: Location) { + fn visit_local( + &mut self, + local: &Local, + context: PlaceContext, + has_projections: bool, + _location: Location, + ) { // Skip the lvalue for assignments if let StatementKind::Assign(box (p, _)) = self.statement.kind { - if p.local == *local && context.is_place_assignment() { + if p.local == *local && context.is_place_assignment() && !has_projections { return; } } @@ -526,7 +536,7 @@ impl<'tcx> MutVisitor<'tcx> for LocalUpdater<'tcx> { self.tcx } - fn visit_local(&mut self, l: &mut Local, _: PlaceContext, _: Location) { + fn visit_local(&mut self, l: &mut Local, _: PlaceContext, _: bool, _: Location) { *l = self.map[*l].unwrap(); } } diff --git a/src/librustc_mir/util/collect_writes.rs b/src/librustc_mir/util/collect_writes.rs index ecf3b08a96eed..257f479bc4696 100644 --- a/src/librustc_mir/util/collect_writes.rs +++ b/src/librustc_mir/util/collect_writes.rs @@ -24,7 +24,13 @@ struct FindLocalAssignmentVisitor { } impl<'tcx> Visitor<'tcx> for FindLocalAssignmentVisitor { - fn visit_local(&mut self, local: &Local, place_context: PlaceContext, location: Location) { + fn visit_local( + &mut self, + local: &Local, + place_context: PlaceContext, + _: bool, + location: Location, + ) { if self.needle != *local { return; } diff --git a/src/librustc_mir/util/def_use.rs b/src/librustc_mir/util/def_use.rs index b4448ead8eb81..81ca8d863d7c8 100644 --- a/src/librustc_mir/util/def_use.rs +++ b/src/librustc_mir/util/def_use.rs @@ -87,7 +87,13 @@ struct DefUseFinder { } impl Visitor<'_> for DefUseFinder { - fn visit_local(&mut self, &local: &Local, context: PlaceContext, location: Location) { + fn visit_local( + &mut self, + &local: &Local, + context: PlaceContext, + _has_projections: bool, + location: Location, + ) { let info = &mut self.info[local]; if self.in_var_debug_info { info.var_debug_info_indices.push(self.var_debug_info_index); @@ -150,7 +156,13 @@ impl MutVisitor<'tcx> for MutateUseVisitor<'tcx> { self.tcx } - fn visit_local(&mut self, local: &mut Local, _context: PlaceContext, _location: Location) { + fn visit_local( + &mut self, + local: &mut Local, + _context: PlaceContext, + _: bool, + _location: Location, + ) { if *local == self.query { *local = self.new_local; }