diff --git a/compiler/rustc_borrowck/src/def_use.rs b/compiler/rustc_borrowck/src/def_use.rs index a5c0d77429de8..e7ea26613b00a 100644 --- a/compiler/rustc_borrowck/src/def_use.rs +++ b/compiler/rustc_borrowck/src/def_use.rs @@ -7,6 +7,10 @@ pub enum DefUse { Def, Use, Drop, + /// This is only created for a `DropAndReplace` terminator. + /// It needs special handling because represents a `Drop` immediately followed + /// by a `Def`, at the same MIR location. + DropAndDef, } pub fn categorize(context: PlaceContext) -> Option { @@ -31,6 +35,8 @@ pub fn categorize(context: PlaceContext) -> Option { PlaceContext::NonUse(NonUseContext::StorageLive) | PlaceContext::NonUse(NonUseContext::StorageDead) => Some(DefUse::Def), + PlaceContext::MutatingUse(MutatingUseContext::DropAndReplace) => Some(DefUse::DropAndDef), + /////////////////////////////////////////////////////////////////////////// // REGULAR USES // diff --git a/compiler/rustc_borrowck/src/diagnostics/find_use.rs b/compiler/rustc_borrowck/src/diagnostics/find_use.rs index b5a3081e56a7a..d0d4204aa2285 100644 --- a/compiler/rustc_borrowck/src/diagnostics/find_use.rs +++ b/compiler/rustc_borrowck/src/diagnostics/find_use.rs @@ -121,6 +121,10 @@ impl<'cx, 'tcx> Visitor<'tcx> for DefUseVisitor<'cx, 'tcx> { Some(DefUse::Def) => Some(DefUseResult::Def), Some(DefUse::Use) => Some(DefUseResult::UseLive { local }), Some(DefUse::Drop) => Some(DefUseResult::UseDrop { local }), + // This makes some diagnostics nicer - we want to indicate + // that a variable is being kept alive because it can be accessed + // by this drop. + Some(DefUse::DropAndDef) => Some(DefUseResult::UseDrop { local }), None => None, }; } 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 fda2cee43fbf1..a3b12d40f1933 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 @@ -24,6 +24,9 @@ pub(crate) struct LocalUseMap { /// defined in `x = y` but not `y`; that first def is the head of /// a linked list that lets you enumerate all places the variable /// is assigned. + /// + /// Note that a Local can have *both* a definition and a drop + /// at the same point - this occurs with `DropAndReplace` terminators. first_def_at: IndexVec>, /// Head of a linked list of **uses** of each variable -- use in @@ -35,6 +38,9 @@ pub(crate) struct LocalUseMap { /// Head of a linked list of **drops** of each variable -- these /// are a special category of uses corresponding to the drop that /// we add for each local variable. + /// + /// Note that a Local can have *both* a definition and a drop + /// at the same point - this occurs with `DropAndReplace` terminators. first_drop_at: IndexVec>, appearances: IndexVec, @@ -163,7 +169,19 @@ impl Visitor<'_> for LocalUseMapBuild<'_> { Some(DefUse::Def) => self.insert_def(local, location), Some(DefUse::Use) => self.insert_use(local, location), Some(DefUse::Drop) => self.insert_drop(local, location), - _ => (), + // This counts as *both* a drop and a def. + // + // Treating it as a *drop* ensures that we consider the local to be + // "drop live" here, which keeps alive any data that the `Drop` impl + // could access. + // + // Treating it as a *def* ensures that we don't create an unnecessarily large "use live" + // set - we'll stop tracing backwards from a use when we hit this def. + Some(DefUse::DropAndDef) => { + self.insert_drop(local, location); + self.insert_def(local, location); + } + None => {} } } } diff --git a/compiler/rustc_codegen_ssa/src/mir/analyze.rs b/compiler/rustc_codegen_ssa/src/mir/analyze.rs index c7617d2e464fa..200e1d384b407 100644 --- a/compiler/rustc_codegen_ssa/src/mir/analyze.rs +++ b/compiler/rustc_codegen_ssa/src/mir/analyze.rs @@ -211,6 +211,7 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> PlaceContext::MutatingUse( MutatingUseContext::Store + | MutatingUseContext::DropAndReplace | MutatingUseContext::Deinit | MutatingUseContext::SetDiscriminant | MutatingUseContext::AsmOutput diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index ddcf3711bfc95..82030584b327a 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -504,7 +504,7 @@ macro_rules! make_mir_visitor { } => { self.visit_place( place, - PlaceContext::MutatingUse(MutatingUseContext::Drop), + PlaceContext::MutatingUse(MutatingUseContext::DropAndReplace), location ); self.visit_operand(value, location); @@ -1267,6 +1267,8 @@ pub enum MutatingUseContext { Yield, /// Being dropped. Drop, + /// A `DropAndReplace` terminator + DropAndReplace, /// Mutable borrow. Borrow, /// AddressOf for *mut pointer. diff --git a/compiler/rustc_mir_dataflow/src/impls/liveness.rs b/compiler/rustc_mir_dataflow/src/impls/liveness.rs index 3e08a8799ef9a..4f69e45a20ef2 100644 --- a/compiler/rustc_mir_dataflow/src/impls/liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/liveness.rs @@ -206,6 +206,13 @@ impl DefUse { | PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection) => { unreachable!("A projection could be a def or a use and must be handled separately") } + + PlaceContext::MutatingUse(MutatingUseContext::DropAndReplace) => { + unreachable!( + "DropAndReplace terminators should have been removed by drop elaboration: place {:?}", + place + ) + } } } } diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index cb134a20ea03e..976857c6edbf1 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -932,6 +932,7 @@ impl Visitor<'_> for CanConstProp { // whether they'd be fine right now. MutatingUse(MutatingUseContext::Yield) | MutatingUse(MutatingUseContext::Drop) + | MutatingUse(MutatingUseContext::DropAndReplace) | MutatingUse(MutatingUseContext::Retag) // These can't ever be propagated under any scheme, as we can't reason about indirect // mutation. diff --git a/src/test/ui/borrowck/drop-in-loop.rs b/src/test/ui/borrowck/drop-in-loop.rs new file mode 100644 index 0000000000000..866c27ef20324 --- /dev/null +++ b/src/test/ui/borrowck/drop-in-loop.rs @@ -0,0 +1,24 @@ +// A version of `issue-70919-drop-in-loop`, but without +// the necessary `drop` call. +// +// This should fail to compile, since the `Drop` impl +// for `WrapperWithDrop` could observe the changed +// `base` value. + +struct WrapperWithDrop<'a>(&'a mut bool); +impl<'a> Drop for WrapperWithDrop<'a> { + fn drop(&mut self) { + } +} + +fn drop_in_loop() { + let mut base = true; + let mut wrapper = WrapperWithDrop(&mut base); + loop { + base = false; //~ ERROR: cannot assign to `base` + wrapper = WrapperWithDrop(&mut base); + } +} + +fn main() { +} diff --git a/src/test/ui/borrowck/drop-in-loop.stderr b/src/test/ui/borrowck/drop-in-loop.stderr new file mode 100644 index 0000000000000..31e5108508137 --- /dev/null +++ b/src/test/ui/borrowck/drop-in-loop.stderr @@ -0,0 +1,14 @@ +error[E0506]: cannot assign to `base` because it is borrowed + --> $DIR/drop-in-loop.rs:18:9 + | +LL | let mut wrapper = WrapperWithDrop(&mut base); + | --------- borrow of `base` occurs here +LL | loop { +LL | base = false; + | ^^^^^^^^^^^^ assignment to borrowed `base` occurs here +LL | wrapper = WrapperWithDrop(&mut base); + | ------- borrow might be used here, when `wrapper` is dropped and runs the `Drop` code for type `WrapperWithDrop` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0506`. diff --git a/src/test/ui/borrowck/issue-70919-drop-in-loop.rs b/src/test/ui/borrowck/issue-70919-drop-in-loop.rs new file mode 100644 index 0000000000000..a8d5849a31c0b --- /dev/null +++ b/src/test/ui/borrowck/issue-70919-drop-in-loop.rs @@ -0,0 +1,25 @@ +// Regression test for issue #70919 +// Tests that we don't emit a spurious "borrow might be used" error +// when we have an explicit `drop` in a loop + +// check-pass + +struct WrapperWithDrop<'a>(&'a mut bool); +impl<'a> Drop for WrapperWithDrop<'a> { + fn drop(&mut self) { + } +} + +fn drop_in_loop() { + let mut base = true; + let mut wrapper = WrapperWithDrop(&mut base); + loop { + drop(wrapper); + + base = false; + wrapper = WrapperWithDrop(&mut base); + } +} + +fn main() { +}