diff --git a/compiler/rustc_borrowck/src/def_use.rs b/compiler/rustc_borrowck/src/def_use.rs index 8e62a0198be46..07aa3f5a91d48 100644 --- a/compiler/rustc_borrowck/src/def_use.rs +++ b/compiler/rustc_borrowck/src/def_use.rs @@ -54,7 +54,7 @@ pub fn categorize(context: PlaceContext) -> Option { PlaceContext::MutatingUse(MutatingUseContext::AddressOf) | PlaceContext::NonMutatingUse(NonMutatingUseContext::AddressOf) | - PlaceContext::NonMutatingUse(NonMutatingUseContext::Inspect) | + PlaceContext::NonMutatingUse(NonMutatingUseContext::Inspect(_)) | PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) | PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) | PlaceContext::NonUse(NonUseContext::AscribeUserTy) | diff --git a/compiler/rustc_borrowck/src/diagnostics/mod.rs b/compiler/rustc_borrowck/src/diagnostics/mod.rs index a75ec87be4cac..079716fbeb7bb 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mod.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mod.rs @@ -939,7 +939,22 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { return OtherUse(use_span); } - for stmt in &self.body[location.block].statements[location.statement_index + 1..] { + // drop and replace might have moved the assignment to the next block + let maybe_additional_statement = if let Some(Terminator { + kind: TerminatorKind::Drop { target: drop_target, .. }, + .. + }) = self.body[location.block].terminator + { + self.body[drop_target].statements.iter().take(1) + } else { + [].iter().take(0) + }; + + let statements = self.body[location.block].statements[location.statement_index + 1..] + .iter() + .chain(maybe_additional_statement); + + for stmt in statements { if let StatementKind::Assign(box (_, Rvalue::Aggregate(kind, places))) = &stmt.kind { let (&def_id, is_generator) = match kind { box AggregateKind::Closure(def_id, _) => (def_id, false), diff --git a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs index 9f37b915b773a..26b438a57f0f9 100644 --- a/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/mutability_errors.rs @@ -828,7 +828,13 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { let Some(hir::Node::Item(item)) = node else { return; }; let hir::ItemKind::Fn(.., body_id) = item.kind else { return; }; let body = self.infcx.tcx.hir().body(body_id); - let mut v = V { assign_span: span, err, ty, suggested: false }; + let mut assign_span = span; + // Drop desugaring is done at MIR build so it's not in the HIR + if let Some(DesugaringKind::Replace) = span.desugaring_kind() { + assign_span.remove_mark(); + } + + let mut v = V { assign_span, err, ty, suggested: false }; v.visit_body(body); if !v.suggested { err.help(&format!( diff --git a/compiler/rustc_borrowck/src/invalidation.rs b/compiler/rustc_borrowck/src/invalidation.rs index 6217676d5c150..70ee4b70d6592 100644 --- a/compiler/rustc_borrowck/src/invalidation.rs +++ b/compiler/rustc_borrowck/src/invalidation.rs @@ -58,7 +58,9 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { match &statement.kind { StatementKind::Assign(box (lhs, rhs)) => { - self.consume_rvalue(location, rhs); + if !matches!(rhs, Rvalue::Discriminant(_)) { + self.consume_rvalue(location, rhs); + } self.mutate_place(location, *lhs, Shallow(None)); } @@ -119,13 +121,12 @@ impl<'cx, 'tcx> Visitor<'tcx> for InvalidationGenerator<'cx, 'tcx> { ); } TerminatorKind::DropAndReplace { - place: drop_place, - value: new_value, + place: _drop_place, + value: _new_value, target: _, unwind: _, } => { - self.mutate_place(location, *drop_place, Deep); - self.consume_operand(location, new_value); + bug!("undesugared drop and replace in borrowck") } TerminatorKind::Call { func, diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index 98103af779d8b..f2e0780b4a698 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -36,7 +36,7 @@ use rustc_middle::mir::{InlineAsmOperand, Terminator, TerminatorKind}; use rustc_middle::ty::query::Providers; use rustc_middle::ty::{self, CapturedPlace, ParamEnv, RegionVid, TyCtxt}; use rustc_session::lint::builtin::UNUSED_MUT; -use rustc_span::{Span, Symbol}; +use rustc_span::{DesugaringKind, Span, Symbol}; use either::Either; use smallvec::SmallVec; @@ -576,7 +576,11 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx match &stmt.kind { StatementKind::Assign(box (lhs, rhs)) => { - self.consume_rvalue(location, (rhs, span), flow_state); + // FIXME: drop-elaboration checks the discriminant of an enum after it has been + // moved out. This is a known isses and should be fixed in the future. + if !(matches!(rhs, Rvalue::Discriminant(_)) && span.desugaring_kind() == Some(DesugaringKind::Drop)) { + self.consume_rvalue(location, (rhs, span), flow_state); + } self.mutate_place(location, (*lhs, span), Shallow(None), flow_state); } @@ -599,7 +603,7 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx ); } StatementKind::Intrinsic(box kind) => match kind { - NonDivergingIntrinsic::Assume(op) => self.consume_operand(location, (op, span), flow_state), + NonDivergingIntrinsic::Assume(op) => self.consume_operand(location, (op, span), flow_state, RequireInit::Yes), NonDivergingIntrinsic::CopyNonOverlapping(..) => span_bug!( span, "Unexpected CopyNonOverlapping, should only appear after lower_intrinsics", @@ -643,7 +647,7 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx match &term.kind { TerminatorKind::SwitchInt { discr, targets: _ } => { - self.consume_operand(loc, (discr, span), flow_state); + self.consume_operand(loc, (discr, span), flow_state, RequireInit::Yes); } TerminatorKind::Drop { place, target: _, unwind: _ } => { debug!( @@ -661,13 +665,12 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx ); } TerminatorKind::DropAndReplace { - place: drop_place, - value: new_value, + place: _drop_place, + value: _new_value, target: _, unwind: _, } => { - self.mutate_place(loc, (*drop_place, span), Deep, flow_state); - self.consume_operand(loc, (new_value, span), flow_state); + bug!("undesugared drop and replace in borrowck") } TerminatorKind::Call { func, @@ -678,23 +681,44 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx from_hir_call: _, fn_span: _, } => { - self.consume_operand(loc, (func, span), flow_state); + self.mutate_place(loc, (*destination, span), Deep, flow_state); + self.consume_operand(loc, (func, span), flow_state, RequireInit::Yes); + + // the initialization dataflow analysis can't understand that drop elaboration + // inserts checks to determine whether a target is initialized before calling + // box_free. In other words, it can't correlate the value of the drop flag with + // the initialization status of the target. + // As a temporary fix, just trust that it does the right thing. + // Note that this is the same behavior of Drop actually, where the borrow checker + // doesn't check that the target is initialized and relies on drop elab to + // remove uninitialized ones. + let func_ty = func.ty(self.body, self.infcx.tcx); + use rustc_hir::lang_items::LangItem; + let require_init = match func_ty.kind() { + ty::FnDef(func_id, _) + if Some(func_id) + == self.infcx.tcx.lang_items().get(LangItem::BoxFree).as_ref() => + { + RequireInit::No + } + _ => RequireInit::Yes, + }; + for arg in args { - self.consume_operand(loc, (arg, span), flow_state); + self.consume_operand(loc, (arg, span), flow_state, require_init); } - self.mutate_place(loc, (*destination, span), Deep, flow_state); } TerminatorKind::Assert { cond, expected: _, msg, target: _, cleanup: _ } => { - self.consume_operand(loc, (cond, span), flow_state); + self.consume_operand(loc, (cond, span), flow_state, RequireInit::Yes); use rustc_middle::mir::AssertKind; if let AssertKind::BoundsCheck { len, index } = msg { - self.consume_operand(loc, (len, span), flow_state); - self.consume_operand(loc, (index, span), flow_state); + self.consume_operand(loc, (len, span), flow_state, RequireInit::Yes); + self.consume_operand(loc, (index, span), flow_state, RequireInit::Yes); } } TerminatorKind::Yield { value, resume: _, resume_arg, drop: _ } => { - self.consume_operand(loc, (value, span), flow_state); + self.consume_operand(loc, (value, span), flow_state, RequireInit::Yes); self.mutate_place(loc, (*resume_arg, span), Deep, flow_state); } @@ -709,7 +733,7 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx for op in operands { match op { InlineAsmOperand::In { reg: _, value } => { - self.consume_operand(loc, (value, span), flow_state); + self.consume_operand(loc, (value, span), flow_state, RequireInit::Yes); } InlineAsmOperand::Out { reg: _, late: _, place, .. } => { if let Some(place) = place { @@ -717,7 +741,12 @@ impl<'cx, 'tcx> rustc_mir_dataflow::ResultsVisitor<'cx, 'tcx> for MirBorrowckCtx } } InlineAsmOperand::InOut { reg: _, late: _, in_value, out_place } => { - self.consume_operand(loc, (in_value, span), flow_state); + self.consume_operand( + loc, + (in_value, span), + flow_state, + RequireInit::Yes, + ); if let &Some(out_place) = out_place { self.mutate_place( loc, @@ -860,6 +889,14 @@ enum WriteKind { Move, } +/// Whether to require a place to be initialized +/// when checking permissions. +#[derive(Copy, Clone, PartialEq, Eq, Debug)] +enum RequireInit { + Yes, + No, +} + /// When checking permissions for a place access, this flag is used to indicate that an immutable /// local place can be mutated. // @@ -1100,13 +1137,23 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { this.report_conflicting_borrow(location, place_span, bk, borrow); this.buffer_error(err); } - WriteKind::StorageDeadOrDrop => this - .report_borrowed_value_does_not_live_long_enough( - location, - borrow, - place_span, - Some(kind), - ), + WriteKind::StorageDeadOrDrop => { + use rustc_span::DesugaringKind; + if let Some(DesugaringKind::Replace) = place_span.1.desugaring_kind() { + // If this is a drop triggered by a reassignment, it's more user friendly + // to report a problem with the explicit assignment than the implicit drop. + this.report_illegal_mutation_of_borrowed( + location, place_span, borrow, + ) + } else { + this.report_borrowed_value_does_not_live_long_enough( + location, + borrow, + place_span, + Some(kind), + ) + } + } WriteKind::Mutate => { this.report_illegal_mutation_of_borrowed(location, place_span, borrow) } @@ -1220,7 +1267,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { | Rvalue::UnaryOp(_ /*un_op*/, operand) | Rvalue::Cast(_ /*cast_kind*/, operand, _ /*ty*/) | Rvalue::ShallowInitBox(operand, _ /*ty*/) => { - self.consume_operand(location, (operand, span), flow_state) + self.consume_operand(location, (operand, span), flow_state, RequireInit::Yes) } &Rvalue::CopyForDeref(place) => { @@ -1264,8 +1311,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { Rvalue::BinaryOp(_bin_op, box (operand1, operand2)) | Rvalue::CheckedBinaryOp(_bin_op, box (operand1, operand2)) => { - self.consume_operand(location, (operand1, span), flow_state); - self.consume_operand(location, (operand2, span), flow_state); + self.consume_operand(location, (operand1, span), flow_state, RequireInit::Yes); + self.consume_operand(location, (operand2, span), flow_state, RequireInit::Yes); } Rvalue::NullaryOp(_op, _ty) => { @@ -1292,7 +1339,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } for operand in operands { - self.consume_operand(location, (operand, span), flow_state); + self.consume_operand(location, (operand, span), flow_state, RequireInit::Yes); } } } @@ -1398,6 +1445,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { location: Location, (operand, span): (&'cx Operand<'tcx>, Span), flow_state: &Flows<'cx, 'tcx>, + require_init: RequireInit, ) { match *operand { Operand::Copy(place) => { @@ -1411,13 +1459,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { 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, - ); + if let RequireInit::Yes = require_init { + // Finally, check if path was already moved. + self.check_if_path_or_subpath_is_moved( + location, + InitializationRequiringAction::Use, + (place.as_ref(), span), + flow_state, + ); + } } Operand::Move(place) => { // move of place: check if this is move of already borrowed path @@ -1429,13 +1479,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { 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, - ); + if let RequireInit::Yes = require_init { + // Finally, check if path was already moved. + self.check_if_path_or_subpath_is_moved( + location, + InitializationRequiringAction::Use, + (place.as_ref(), span), + flow_state, + ); + } } Operand::Constant(_) => {} } diff --git a/compiler/rustc_borrowck/src/type_check/liveness/local_use_map.rs b/compiler/rustc_borrowck/src/type_check/liveness/local_use_map.rs index 8023ef60d2052..2d4b097154cb7 100644 --- a/compiler/rustc_borrowck/src/type_check/liveness/local_use_map.rs +++ b/compiler/rustc_borrowck/src/type_check/liveness/local_use_map.rs @@ -1,6 +1,6 @@ use rustc_data_structures::vec_linked_list as vll; use rustc_index::vec::IndexVec; -use rustc_middle::mir::visit::{PlaceContext, Visitor}; +use rustc_middle::mir::visit::{NonMutatingUseContext, PlaceContext, Visitor}; use rustc_middle::mir::{Body, Local, Location}; use crate::def_use::{self, DefUse}; @@ -158,6 +158,20 @@ impl LocalUseMapBuild<'_> { impl Visitor<'_> for LocalUseMapBuild<'_> { fn visit_local(&mut self, local: Local, context: PlaceContext, location: Location) { + if matches!( + context, + PlaceContext::NonMutatingUse(NonMutatingUseContext::Inspect( + rustc_middle::mir::visit::InspectKind::Discriminant + )) + ) { + // drop elaboration inserts discriminant reads for enums, but such reads should + // really be counted as a drop access for liveness computation. + // However, doing so messes with the way we calculate drop points, so for now just ignore + // those, as discriminant reads are ignored anyway by the borrowck. + // FIXME: found a better solution for drop-elab discriminant reads + return; + } + if self.locals_with_use_data[local] { match def_use::categorize(context) { Some(DefUse::Def) => self.insert_def(local, location), diff --git a/compiler/rustc_codegen_ssa/src/mir/analyze.rs b/compiler/rustc_codegen_ssa/src/mir/analyze.rs index 95aad10fdb0f9..71f970fb44989 100644 --- a/compiler/rustc_codegen_ssa/src/mir/analyze.rs +++ b/compiler/rustc_codegen_ssa/src/mir/analyze.rs @@ -230,7 +230,7 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> | MutatingUseContext::Projection, ) | PlaceContext::NonMutatingUse( - NonMutatingUseContext::Inspect + NonMutatingUseContext::Inspect(_) | NonMutatingUseContext::SharedBorrow | NonMutatingUseContext::UniqueBorrow | NonMutatingUseContext::ShallowBorrow diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index 3ddac5e11fbc5..7df1b8b534381 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -370,7 +370,7 @@ macro_rules! make_mir_visitor { StatementKind::FakeRead(box (_, place)) => { self.visit_place( place, - PlaceContext::NonMutatingUse(NonMutatingUseContext::Inspect), + PlaceContext::NonMutatingUse(NonMutatingUseContext::Inspect(InspectKind::Other)), location ); } @@ -652,7 +652,7 @@ macro_rules! make_mir_visitor { Rvalue::CopyForDeref(place) => { self.visit_place( place, - PlaceContext::NonMutatingUse(NonMutatingUseContext::Inspect), + PlaceContext::NonMutatingUse(NonMutatingUseContext::Inspect(InspectKind::Other)), location ); } @@ -672,7 +672,7 @@ macro_rules! make_mir_visitor { Rvalue::Len(path) => { self.visit_place( path, - PlaceContext::NonMutatingUse(NonMutatingUseContext::Inspect), + PlaceContext::NonMutatingUse(NonMutatingUseContext::Inspect(InspectKind::Other)), location ); } @@ -695,7 +695,7 @@ macro_rules! make_mir_visitor { Rvalue::Discriminant(place) => { self.visit_place( place, - PlaceContext::NonMutatingUse(NonMutatingUseContext::Inspect), + PlaceContext::NonMutatingUse(NonMutatingUseContext::Inspect(InspectKind::Discriminant)), location ); } @@ -1132,7 +1132,7 @@ macro_rules! visit_place_fns { let mut context = context; if !place.projection.is_empty() { - if context.is_use() { + if context.is_use() && !context.is_drop() { // ^ Only change the context if it is a real use, not a "use" in debuginfo. context = if context.is_mutating_use() { PlaceContext::MutatingUse(MutatingUseContext::Projection) @@ -1236,10 +1236,16 @@ pub enum TyContext { Location(Location), } +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +pub enum InspectKind { + Other, + Discriminant, +} + #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum NonMutatingUseContext { /// Being inspected in some way, like loading a len. - Inspect, + Inspect(InspectKind), /// Consumed as part of an operand. Copy, /// Consumed as part of an operand. diff --git a/compiler/rustc_mir_build/src/build/expr/stmt.rs b/compiler/rustc_mir_build/src/build/expr/stmt.rs index 7808368519351..a8766f6f4553f 100644 --- a/compiler/rustc_mir_build/src/build/expr/stmt.rs +++ b/compiler/rustc_mir_build/src/build/expr/stmt.rs @@ -39,10 +39,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Generate better code for things that don't need to be // dropped. + use rustc_span::DesugaringKind; if lhs.ty.needs_drop(this.tcx, this.param_env) { let rhs = unpack!(block = this.as_local_operand(block, rhs)); let lhs = unpack!(block = this.as_place(block, lhs)); - unpack!(block = this.build_drop_and_replace(block, lhs_span, lhs, rhs)); + let span = this.tcx.with_stable_hashing_context(|hcx| { + lhs_span.mark_with_reason( + None, + DesugaringKind::Replace, + this.tcx.sess.edition(), + hcx, + ) + }); + unpack!(block = this.build_drop_and_replace(block, span, lhs, rhs)); } else { let rhs = unpack!(block = this.as_local_rvalue(block, rhs)); let lhs = unpack!(block = this.as_place(block, lhs)); diff --git a/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs b/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs index 3224e13f7af4b..67f0029dd155c 100644 --- a/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs +++ b/compiler/rustc_mir_dataflow/src/drop_flag_effects.rs @@ -29,6 +29,35 @@ where None } +// Drop elaboration of a Box makes the inner fields (pointer and allocator) explicit in MIR, +// which causes those path to be tracked by dataflow and checked by the borrowck. +// However, those are not considered to be children of *_box_place (and only of _box_place), +// which causes issues in initialization analysis when accessing _box_place. +// This code basically check if the parent of the path is a box and if so, apply the callback +// to its children to fix such cases. +fn maybe_recurse_box_parent<'tcx, F>( + tcx: TyCtxt<'tcx>, + body: &Body<'tcx>, + move_data: &MoveData<'tcx>, + move_path_index: MovePathIndex, + mut each_child: F, +) where + F: FnMut(MovePathIndex), +{ + if let Some(path) = move_data.move_paths[move_path_index].parent { + let place = move_data.move_paths[path].place; + let ty = place.ty(body, tcx).ty; + if let ty::Adt(def, _) = ty.kind() { + if def.is_box() { + each_child(move_path_index); + on_all_children_bits(tcx, body, move_data, path, each_child); + return; + } + } + } + on_all_children_bits(tcx, body, move_data, move_path_index, each_child); +} + pub fn on_lookup_result_bits<'tcx, F>( tcx: TyCtxt<'tcx>, body: &Body<'tcx>, @@ -42,7 +71,7 @@ pub fn on_lookup_result_bits<'tcx, F>( LookupResult::Parent(..) => { // access to untracked value - do not touch children } - LookupResult::Exact(e) => on_all_children_bits(tcx, body, move_data, e, each_child), + LookupResult::Exact(e) => maybe_recurse_box_parent(tcx, body, move_data, e, each_child), } } @@ -194,6 +223,19 @@ pub fn drop_flag_effects_for_location<'tcx, F>( on_all_children_bits(tcx, body, move_data, path, |mpi| callback(mpi, DropFlagState::Absent)) } + use rustc_middle::mir::{Terminator, TerminatorKind}; + + // Drop does not count as a move but we should still consider the variable uninitialized. + if let Some(Terminator { kind: TerminatorKind::Drop { place, .. }, .. }) = + body.stmt_at(loc).right() + { + if let LookupResult::Exact(mpi) = move_data.rev_lookup.find(place.as_ref()) { + on_all_children_bits(tcx, body, move_data, mpi, |mpi| { + callback(mpi, DropFlagState::Absent) + }) + } + } + debug!("drop_flag_effects: assignment for location({:?})", loc); for_location_inits(tcx, body, move_data, loc, |mpi| callback(mpi, DropFlagState::Present)); @@ -212,9 +254,7 @@ pub fn for_location_inits<'tcx, F>( let init = move_data.inits[*ii]; match init.kind { InitKind::Deep => { - let path = init.path; - - on_all_children_bits(tcx, body, move_data, path, &mut callback) + maybe_recurse_box_parent(tcx, body, move_data, init.path, &mut callback) } InitKind::Shallow => { let mpi = init.path; diff --git a/compiler/rustc_mir_dataflow/src/elaborate_drops.rs b/compiler/rustc_mir_dataflow/src/elaborate_drops.rs index 7836ae2e7b76f..ded0308bbab52 100644 --- a/compiler/rustc_mir_dataflow/src/elaborate_drops.rs +++ b/compiler/rustc_mir_dataflow/src/elaborate_drops.rs @@ -3,7 +3,6 @@ use rustc_hir::lang_items::LangItem; use rustc_index::vec::Idx; use rustc_middle::mir::patch::MirPatch; use rustc_middle::mir::*; -use rustc_middle::traits::Reveal; use rustc_middle::ty::subst::SubstsRef; use rustc_middle::ty::util::IntTypeExt; use rustc_middle::ty::{self, Ty, TyCtxt}; @@ -186,6 +185,16 @@ pub fn elaborate_drop<'b, 'tcx, D>( D: DropElaborator<'b, 'tcx>, 'tcx: 'b, { + use rustc_span::DesugaringKind; + let span = elaborator.tcx().with_stable_hashing_context(|hcx| { + source_info.span.mark_with_reason( + None, + DesugaringKind::Drop, + elaborator.tcx().sess.edition(), + hcx, + ) + }); + let source_info = SourceInfo { span, ..source_info }; DropCtxt { elaborator, source_info, place, path, succ, unwind }.elaborate_drop(bb) } @@ -273,7 +282,6 @@ where let subpath = self.elaborator.field_subpath(variant_path, field); let tcx = self.tcx(); - assert_eq!(self.elaborator.param_env().reveal(), Reveal::All); let field_ty = tcx.normalize_erasing_regions(self.elaborator.param_env(), f.ty(tcx, substs)); (tcx.mk_place_field(base_place, field, field_ty), subpath) diff --git a/compiler/rustc_mir_dataflow/src/impls/liveness.rs b/compiler/rustc_mir_dataflow/src/impls/liveness.rs index 2890fa32cc915..d468ec3104bff 100644 --- a/compiler/rustc_mir_dataflow/src/impls/liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/liveness.rs @@ -195,7 +195,7 @@ impl DefUse { | PlaceContext::NonMutatingUse( NonMutatingUseContext::AddressOf | NonMutatingUseContext::Copy - | NonMutatingUseContext::Inspect + | NonMutatingUseContext::Inspect(_) | NonMutatingUseContext::Move | NonMutatingUseContext::ShallowBorrow | NonMutatingUseContext::SharedBorrow diff --git a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs index 0195693a7cb0e..55e45189045c0 100644 --- a/compiler/rustc_mir_dataflow/src/move_paths/builder.rs +++ b/compiler/rustc_mir_dataflow/src/move_paths/builder.rs @@ -376,7 +376,8 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { | TerminatorKind::Resume | TerminatorKind::Abort | TerminatorKind::GeneratorDrop - | TerminatorKind::Unreachable => {} + | TerminatorKind::Unreachable + | TerminatorKind::Drop { .. } => {} TerminatorKind::Assert { ref cond, .. } => { self.gather_operand(cond); @@ -391,10 +392,6 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { self.create_move_path(place); self.gather_init(place.as_ref(), InitKind::Deep); } - - TerminatorKind::Drop { place, target: _, unwind: _ } => { - self.gather_move(place); - } TerminatorKind::DropAndReplace { place, ref value, .. } => { self.create_move_path(place); self.gather_operand(value); @@ -410,13 +407,26 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> { fn_span: _, } => { self.gather_operand(func); - for arg in args { - self.gather_operand(arg); - } + if let Some(_bb) = target { self.create_move_path(destination); self.gather_init(destination.as_ref(), InitKind::NonPanicPathOnly); } + + // drop glue for nested boxes moves inner pointers in a way that is invalid according + // to move analysis. This is a hack to avoid triggering an error. + let func_ty = func.ty(self.builder.body, self.builder.tcx); + use rustc_hir::lang_items::LangItem; + if let ty::FnDef(func_id, _) = func_ty.kind() { + if Some(func_id) == self.builder.tcx.lang_items().get(LangItem::BoxFree).as_ref() + { + return; + } + } + + for arg in args { + self.gather_operand(arg); + } } TerminatorKind::InlineAsm { template: _, diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index feb054392bc2d..71143ca5bd0b6 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -934,7 +934,7 @@ impl Visitor<'_> for CanConstProp { // Reading constants is allowed an arbitrary number of times NonMutatingUse(NonMutatingUseContext::Copy) | NonMutatingUse(NonMutatingUseContext::Move) - | NonMutatingUse(NonMutatingUseContext::Inspect) + | NonMutatingUse(NonMutatingUseContext::Inspect(_)) | NonMutatingUse(NonMutatingUseContext::Projection) | NonUse(_) => {} diff --git a/compiler/rustc_mir_transform/src/elaborate_drops.rs b/compiler/rustc_mir_transform/src/elaborate_drops.rs index 65f4956d23acd..36488f36fa7e4 100644 --- a/compiler/rustc_mir_transform/src/elaborate_drops.rs +++ b/compiler/rustc_mir_transform/src/elaborate_drops.rs @@ -1,4 +1,4 @@ -use crate::deref_separator::deref_finder; +use crate::simplify::remove_dead_blocks; use crate::MirPass; use rustc_data_structures::fx::FxHashMap; use rustc_index::bit_set::BitSet; @@ -25,7 +25,7 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops { debug!("elaborate_drops({:?} @ {:?})", body.source, body.span); let def_id = body.source.def_id(); - let param_env = tcx.param_env_reveal_all_normalized(def_id); + let param_env = tcx.param_env(def_id); let (side_table, move_data) = match MoveData::gather_moves(body, tcx, param_env) { Ok(move_data) => move_data, Err((move_data, _)) => { @@ -69,7 +69,9 @@ impl<'tcx> MirPass<'tcx> for ElaborateDrops { .elaborate() }; elaborate_patch.apply(body); - deref_finder(tcx, body); + + remove_dead_blocks(tcx, body); + body.basic_blocks_mut().raw.shrink_to_fit(); } } @@ -425,7 +427,6 @@ impl<'b, 'tcx> ElaborateDropsCtxt<'b, 'tcx> { let data = &self.body[bb]; let terminator = data.terminator(); assert!(!data.is_cleanup, "DropAndReplace in unwind path not supported"); - let assign = Statement { kind: StatementKind::Assign(Box::new((place, Rvalue::Use(value.clone())))), source_info: terminator.source_info, diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 9070a7368b168..3d0e95d96877b 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -339,6 +339,9 @@ fn mir_promoted( &[ &promote_pass, &simplify::SimplifyCfg::new("promote-consts"), + // These next passes must be executed together + &add_call_guards::CriticalCallEdges, + &elaborate_drops::ElaborateDrops, &coverage::InstrumentCoverage, ], Some(MirPhase::Analysis(AnalysisPhase::Initial)), @@ -507,9 +510,6 @@ fn run_analysis_cleanup_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { /// Returns the sequence of passes that lowers analysis to runtime MIR. fn run_runtime_lowering_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { let passes: &[&dyn MirPass<'tcx>] = &[ - // These next passes must be executed together - &add_call_guards::CriticalCallEdges, - &elaborate_drops::ElaborateDrops, // This will remove extraneous landing pads which are no longer // necessary as well as well as forcing any call in a non-unwinding // function calling a possibly-unwinding function to abort the process. diff --git a/compiler/rustc_span/src/hygiene.rs b/compiler/rustc_span/src/hygiene.rs index dee823eefde68..04395e4aab434 100644 --- a/compiler/rustc_span/src/hygiene.rs +++ b/compiler/rustc_span/src/hygiene.rs @@ -1151,6 +1151,9 @@ pub enum DesugaringKind { Await, ForLoop, WhileLoop, + Replace, + /// Code inserted by the compiler as part of drop elaboration + Drop, } impl DesugaringKind { @@ -1166,6 +1169,8 @@ impl DesugaringKind { DesugaringKind::OpaqueTy => "`impl Trait`", DesugaringKind::ForLoop => "`for` loop", DesugaringKind::WhileLoop => "`while` loop", + DesugaringKind::Replace => "drop and replace", + DesugaringKind::Drop => "drop elaboration", } } } diff --git a/tests/ui/borrowck/borrowck-vec-pattern-nesting.rs b/tests/ui/borrowck/borrowck-vec-pattern-nesting.rs index 1bda7a4971375..f1d2e9ad26d03 100644 --- a/tests/ui/borrowck/borrowck-vec-pattern-nesting.rs +++ b/tests/ui/borrowck/borrowck-vec-pattern-nesting.rs @@ -1,6 +1,5 @@ #![feature(box_patterns)] - fn a() { let mut vec = [Box::new(1), Box::new(2), Box::new(3)]; match vec { @@ -8,6 +7,7 @@ fn a() { //~^ NOTE `vec[_]` is borrowed here vec[0] = Box::new(4); //~ ERROR cannot assign //~^ NOTE `vec[_]` is assigned to here + //~| NOTE in this expansion of desugaring of drop and replace _a.use_ref(); //~^ NOTE borrow later used here } @@ -22,6 +22,7 @@ fn b() { //~^ `vec[_]` is borrowed here vec[0] = Box::new(4); //~ ERROR cannot assign //~^ NOTE `vec[_]` is assigned to here + //~| NOTE in this expansion of desugaring of drop and replace _b.use_ref(); //~^ NOTE borrow later used here } @@ -44,9 +45,9 @@ fn c() { _ => {} } let a = vec[0]; //~ ERROR cannot move out - //~| NOTE cannot move out of here - //~| NOTE move occurs because - //~| HELP consider borrowing here + //~| NOTE cannot move out of here + //~| NOTE move occurs because + //~| HELP consider borrowing here } fn d() { @@ -63,9 +64,9 @@ fn d() { _ => {} } let a = vec[0]; //~ ERROR cannot move out - //~| NOTE cannot move out of here - //~| NOTE move occurs because - //~| HELP consider borrowing here + //~| NOTE cannot move out of here + //~| NOTE move occurs because + //~| HELP consider borrowing here } fn e() { @@ -83,12 +84,15 @@ fn e() { _ => {} } let a = vec[0]; //~ ERROR cannot move out - //~| NOTE cannot move out of here - //~| NOTE move occurs because - //~| HELP consider borrowing here + //~| NOTE cannot move out of here + //~| NOTE move occurs because + //~| HELP consider borrowing here } fn main() {} -trait Fake { fn use_mut(&mut self) { } fn use_ref(&self) { } } -impl Fake for T { } +trait Fake { + fn use_mut(&mut self) {} + fn use_ref(&self) {} +} +impl Fake for T {} diff --git a/tests/ui/borrowck/borrowck-vec-pattern-nesting.stderr b/tests/ui/borrowck/borrowck-vec-pattern-nesting.stderr index 70b9e4f4433b3..197a90ee6e465 100644 --- a/tests/ui/borrowck/borrowck-vec-pattern-nesting.stderr +++ b/tests/ui/borrowck/borrowck-vec-pattern-nesting.stderr @@ -1,12 +1,12 @@ error[E0506]: cannot assign to `vec[_]` because it is borrowed - --> $DIR/borrowck-vec-pattern-nesting.rs:9:13 + --> $DIR/borrowck-vec-pattern-nesting.rs:8:13 | LL | [box ref _a, _, _] => { | ------ `vec[_]` is borrowed here LL | LL | vec[0] = Box::new(4); | ^^^^^^ `vec[_]` is assigned to here but it was already borrowed -LL | +... LL | _a.use_ref(); | ------------ borrow later used here @@ -18,12 +18,12 @@ LL | &mut [ref _b @ ..] => { LL | LL | vec[0] = Box::new(4); | ^^^^^^ `vec[_]` is assigned to here but it was already borrowed -LL | +... LL | _b.use_ref(); | ------------ borrow later used here error[E0508]: cannot move out of type `[Box]`, a non-copy slice - --> $DIR/borrowck-vec-pattern-nesting.rs:34:11 + --> $DIR/borrowck-vec-pattern-nesting.rs:35:11 | LL | match vec { | ^^^ cannot move out of here @@ -41,7 +41,7 @@ LL + [_a, | error[E0508]: cannot move out of type `[Box]`, a non-copy slice - --> $DIR/borrowck-vec-pattern-nesting.rs:46:13 + --> $DIR/borrowck-vec-pattern-nesting.rs:47:13 | LL | let a = vec[0]; | ^^^^^^ @@ -55,7 +55,7 @@ LL | let a = &vec[0]; | + error[E0508]: cannot move out of type `[Box]`, a non-copy slice - --> $DIR/borrowck-vec-pattern-nesting.rs:55:11 + --> $DIR/borrowck-vec-pattern-nesting.rs:56:11 | LL | match vec { | ^^^ cannot move out of here @@ -73,7 +73,7 @@ LL + [ | error[E0508]: cannot move out of type `[Box]`, a non-copy slice - --> $DIR/borrowck-vec-pattern-nesting.rs:65:13 + --> $DIR/borrowck-vec-pattern-nesting.rs:66:13 | LL | let a = vec[0]; | ^^^^^^ @@ -87,7 +87,7 @@ LL | let a = &vec[0]; | + error[E0508]: cannot move out of type `[Box]`, a non-copy slice - --> $DIR/borrowck-vec-pattern-nesting.rs:74:11 + --> $DIR/borrowck-vec-pattern-nesting.rs:75:11 | LL | match vec { | ^^^ cannot move out of here @@ -106,7 +106,7 @@ LL + [_a, _b, _c] => {} | error[E0508]: cannot move out of type `[Box]`, a non-copy slice - --> $DIR/borrowck-vec-pattern-nesting.rs:85:13 + --> $DIR/borrowck-vec-pattern-nesting.rs:86:13 | LL | let a = vec[0]; | ^^^^^^ diff --git a/tests/ui/borrowck/issue-45199.rs b/tests/ui/borrowck/issue-45199.rs index ded46e56e3451..8d4e966caea46 100644 --- a/tests/ui/borrowck/issue-45199.rs +++ b/tests/ui/borrowck/issue-45199.rs @@ -2,23 +2,27 @@ fn test_drop_replace() { let b: Box; //~^ HELP consider making this binding mutable //~| SUGGESTION mut b - b = Box::new(1); //~ NOTE first assignment - b = Box::new(2); //~ ERROR cannot assign twice to immutable variable `b` - //~| NOTE cannot assign twice to immutable + b = Box::new(1); //~ NOTE first assignment + b = Box::new(2); //~ ERROR cannot assign twice to immutable variable `b` + //~| NOTE cannot assign twice to immutable + //~| NOTE in this expansion of desugaring of drop and replace } fn test_call() { - let b = Box::new(1); //~ NOTE first assignment - //~| HELP consider making this binding mutable - //~| SUGGESTION mut b - b = Box::new(2); //~ ERROR cannot assign twice to immutable variable `b` - //~| NOTE cannot assign twice to immutable + let b = Box::new(1); //~ NOTE first assignment + //~| HELP consider making this binding mutable + //~| SUGGESTION mut b + b = Box::new(2); //~ ERROR cannot assign twice to immutable variable `b` + //~| NOTE cannot assign twice to immutable + //~| NOTE in this expansion of desugaring of drop and replace } -fn test_args(b: Box) { //~ HELP consider making this binding mutable - //~| SUGGESTION mut b - b = Box::new(2); //~ ERROR cannot assign to immutable argument `b` - //~| NOTE cannot assign to immutable argument +fn test_args(b: Box) { //~ HELP consider making this binding mutable + //~| SUGGESTION mut b + + b = Box::new(2); //~ ERROR cannot assign to immutable argument `b` + //~| NOTE cannot assign to immutable argument + //~| NOTE in this expansion of desugaring of drop and replace } fn main() {} diff --git a/tests/ui/borrowck/issue-45199.stderr b/tests/ui/borrowck/issue-45199.stderr index 47aa30908270d..4f59ddf41c833 100644 --- a/tests/ui/borrowck/issue-45199.stderr +++ b/tests/ui/borrowck/issue-45199.stderr @@ -10,7 +10,7 @@ LL | b = Box::new(2); | ^ cannot assign twice to immutable variable error[E0384]: cannot assign twice to immutable variable `b` - --> $DIR/issue-45199.rs:14:5 + --> $DIR/issue-45199.rs:15:5 | LL | let b = Box::new(1); | - @@ -22,11 +22,11 @@ LL | b = Box::new(2); | ^ cannot assign twice to immutable variable error[E0384]: cannot assign to immutable argument `b` - --> $DIR/issue-45199.rs:20:5 + --> $DIR/issue-45199.rs:23:5 | LL | fn test_args(b: Box) { | - help: consider making this binding mutable: `mut b` -LL | +... LL | b = Box::new(2); | ^ cannot assign to immutable argument diff --git a/tests/ui/liveness/liveness-assign/liveness-assign-imm-local-with-drop.rs b/tests/ui/liveness/liveness-assign/liveness-assign-imm-local-with-drop.rs index c9b16e43910e8..ea832ee411cfe 100644 --- a/tests/ui/liveness/liveness-assign/liveness-assign-imm-local-with-drop.rs +++ b/tests/ui/liveness/liveness-assign/liveness-assign-imm-local-with-drop.rs @@ -5,8 +5,8 @@ fn test() { drop(b); b = Box::new(2); //~ ERROR cannot assign twice to immutable variable `b` //~| NOTE cannot assign twice to immutable + //~| NOTE in this expansion of desugaring of drop and replace drop(b); } -fn main() { -} +fn main() {}