]> git.lizzy.rs Git - rust.git/commitdiff
Rollup merge of #107271 - Zeegomo:drop-rmw, r=oli-obk
authorMatthias Krüger <matthias.krueger@famsik.de>
Wed, 8 Feb 2023 17:32:41 +0000 (18:32 +0100)
committerGitHub <noreply@github.com>
Wed, 8 Feb 2023 17:32:41 +0000 (18:32 +0100)
Treat Drop as a rmw operation

Previously, a Drop terminator was considered a move in MIR. This commit changes the behavior to only treat Drop as a mutable access to the dropped place.

In order for this change to be correct, we need to guarantee that

1.  A dropped value won't be used again
   2.  Places that appear in a drop won't be used again before a
     subsequent initialization.

We can ensure this to be correct at MIR construction because Drop will only be emitted when a variable goes out of scope, thus having:
*   (1) as there is no way of reaching the old value. drop-elaboration
     will also remove any uninitialized drop.
 * (2) as the place can't be named following the end of the scope.

However, the initialization status, previously tracked by moves, should also be tied to the execution of a Drop, hence the additional logic in the dataflow analyses.

From discussion in [this thread](https://rust-lang.zulipchat.com/#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/.60DROP.60.20to.20.60DROP_IF.60.20compiler-team.23558), originating from https://github.com/rust-lang/compiler-team/issues/558.
See also https://github.com/rust-lang/rust/pull/104488#discussion_r1085556010

1  2 
compiler/rustc_mir_dataflow/src/move_paths/builder.rs
compiler/rustc_mir_dataflow/src/value_analysis.rs

index 0195693a7cb0e6d9bf8639e83d8e96a5b59797a3,f7f98d6e31cc6f635c66762a1f3fc914ab303f8a..115c8afcce0cf7c15964bd53b175366c0ac6b7fd
@@@ -331,7 -331,6 +331,7 @@@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tc
              | StatementKind::AscribeUserType(..)
              | StatementKind::Coverage(..)
              | StatementKind::Intrinsic(..)
 +            | StatementKind::ConstEvalCounter
              | StatementKind::Nop => {}
          }
      }
              | TerminatorKind::Resume
              | TerminatorKind::Abort
              | TerminatorKind::GeneratorDrop
-             | TerminatorKind::Unreachable => {}
+             | TerminatorKind::Unreachable
+             | TerminatorKind::Drop { .. } => {}
  
              TerminatorKind::Assert { ref cond, .. } => {
                  self.gather_operand(cond);
                  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);
index 8bf6493be4b0168ba6e78d8be9d0cbb7c038382c,8c5cf0e557492470359fa7a6cae2c4f031b859af..a6ef2a742c8736219f84946e85a8e6e44ee2c544
@@@ -84,8 -84,7 +84,8 @@@ pub trait ValueAnalysis<'tcx> 
              StatementKind::Retag(..) => {
                  // We don't track references.
              }
 -            StatementKind::Nop
 +            StatementKind::ConstEvalCounter
 +            | StatementKind::Nop
              | StatementKind::FakeRead(..)
              | StatementKind::Coverage(..)
              | StatementKind::AscribeUserType(..) => (),
          self.super_terminator(terminator, state)
      }
  
-     fn super_terminator(&self, terminator: &Terminator<'tcx>, _state: &mut State<Self::Value>) {
+     fn super_terminator(&self, terminator: &Terminator<'tcx>, state: &mut State<Self::Value>) {
          match &terminator.kind {
              TerminatorKind::Call { .. } | TerminatorKind::InlineAsm { .. } => {
                  // Effect is applied by `handle_call_return`.
              }
-             TerminatorKind::Drop { .. } => {
-                 // We don't track dropped places.
+             TerminatorKind::Drop { place, .. } => {
+                 state.flood_with(place.as_ref(), self.map(), Self::Value::bottom());
              }
              TerminatorKind::DropAndReplace { .. } | TerminatorKind::Yield { .. } => {
                  // They would have an effect, but are not allowed in this phase.
@@@ -790,7 -789,7 +790,7 @@@ impl<V, T> TryFrom<ProjectionElem<V, T>
  }
  
  /// Invokes `f` on all direct fields of `ty`.
 -fn iter_fields<'tcx>(
 +pub fn iter_fields<'tcx>(
      ty: Ty<'tcx>,
      tcx: TyCtxt<'tcx>,
      mut f: impl FnMut(Option<VariantIdx>, Field, Ty<'tcx>),
  }
  
  /// Returns all locals with projections that have their reference or address taken.
 -fn excluded_locals(body: &Body<'_>) -> IndexVec<Local, bool> {
 +pub fn excluded_locals(body: &Body<'_>) -> IndexVec<Local, bool> {
      struct Collector {
          result: IndexVec<Local, bool>,
      }