]> git.lizzy.rs Git - rust.git/commitdiff
Revert "Simplify unscheduling of drops after moves"
authorFelix S. Klock II <pnkfelix@pnkfx.org>
Fri, 22 Jan 2021 03:44:02 +0000 (22:44 -0500)
committerFelix S. Klock II <pnkfelix@pnkfx.org>
Fri, 5 Feb 2021 02:29:50 +0000 (21:29 -0500)
This reverts commit b766abc88f78f36193ddefb1079dbc832346b358.

compiler/rustc_mir_build/src/build/scope.rs
src/test/mir-opt/box_expr.main.ElaborateDrops.before.mir
src/test/mir-opt/issue_41110.main.ElaborateDrops.after.mir
src/test/mir-opt/issue_41110.test.ElaborateDrops.after.mir
src/test/mir-opt/no_spurious_drop_after_call.main.ElaborateDrops.before.mir

index 538ad36a6fd27fc6dedb428fc3f91831c44a074f..03717183707b305571031d3219a598be6850efa1 100644 (file)
@@ -83,7 +83,7 @@
 
 use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder, CFG};
 use crate::thir::{Expr, ExprRef, LintLevel};
-use rustc_data_structures::fx::{FxHashMap, FxHashSet};
+use rustc_data_structures::fx::FxHashMap;
 use rustc_index::vec::IndexVec;
 use rustc_middle::middle::region;
 use rustc_middle::mir::*;
@@ -120,6 +120,8 @@ struct Scope {
     /// end of the vector (top of the stack) first.
     drops: Vec<DropData>,
 
+    moved_locals: Vec<Local>,
+
     /// The drop index that will drop everything in and below this scope on an
     /// unwind path.
     cached_unwind_block: Option<DropIdx>,
@@ -403,6 +405,7 @@ fn push_scope(&mut self, region_scope: (region::Scope, SourceInfo), vis_scope: S
             region_scope: region_scope.0,
             region_scope_span: region_scope.1.span,
             drops: vec![],
+            moved_locals: vec![],
             cached_unwind_block: None,
             cached_generator_drop_block: None,
         });
@@ -890,19 +893,20 @@ fn leave_top_scope(&mut self, block: BasicBlock) -> BasicBlock {
         assert_eq!(scope.region_scope, local_scope, "local scope is not the topmost scope!",);
 
         // look for moves of a local variable, like `MOVE(_X)`
-        let locals_moved = operands
-            .iter()
-            .filter_map(|operand| match operand {
-                Operand::Copy(_) | Operand::Constant(_) => None,
-                Operand::Move(place) => place.as_local(),
-            })
-            .collect::<FxHashSet<_>>();
+        let locals_moved = operands.iter().flat_map(|operand| match operand {
+            Operand::Copy(_) | Operand::Constant(_) => None,
+            Operand::Move(place) => place.as_local(),
+        });
 
-        // Remove the drops for the moved operands.
-        scope
-            .drops
-            .retain(|drop| drop.kind == DropKind::Storage || !locals_moved.contains(&drop.local));
-        scope.invalidate_cache();
+        for local in locals_moved {
+            // check if we have a Drop for this operand and -- if so
+            // -- add it to the list of moved operands. Note that this
+            // local might not have been an operand created for this
+            // call, it could come from other places too.
+            if scope.drops.iter().any(|drop| drop.local == local && drop.kind == DropKind::Value) {
+                scope.moved_locals.push(local);
+            }
+        }
     }
 
     // Other
@@ -1147,6 +1151,14 @@ fn build_scope_drops<'tcx>(
                 debug_assert_eq!(unwind_drops.drops[unwind_to].0.kind, drop_data.kind);
                 unwind_to = unwind_drops.drops[unwind_to].1;
 
+                // If the operand has been moved, and we are not on an unwind
+                // path, then don't generate the drop. (We only take this into
+                // account for non-unwind paths so as not to disturb the
+                // caching mechanism.)
+                if scope.moved_locals.iter().any(|&o| o == local) {
+                    continue;
+                }
+
                 unwind_drops.add_entry(block, unwind_to);
 
                 let next = cfg.start_new_block();
index ce5cb39595269360a0c3cb3664161ca6439ac2c5..cfbd3a58637c0edd08c958bf59739c1b9841d761 100644 (file)
@@ -14,7 +14,7 @@ fn main() -> () {
         StorageLive(_1);                 // scope 0 at $DIR/box_expr.rs:7:9: 7:10
         StorageLive(_2);                 // scope 0 at $DIR/box_expr.rs:7:13: 7:25
         _2 = Box(S);                     // scope 0 at $DIR/box_expr.rs:7:13: 7:25
-        (*_2) = S::new() -> [return: bb1, unwind: bb6]; // scope 0 at $DIR/box_expr.rs:7:17: 7:25
+        (*_2) = S::new() -> [return: bb1, unwind: bb7]; // scope 0 at $DIR/box_expr.rs:7:17: 7:25
                                          // mir::Constant
                                          // + span: $DIR/box_expr.rs:7:17: 7:23
                                          // + literal: Const { ty: fn() -> S {S::new}, val: Value(Scalar(<ZST>)) }
@@ -49,14 +49,18 @@ fn main() -> () {
     }
 
     bb5 (cleanup): {
-        drop(_1) -> bb7;                 // scope 0 at $DIR/box_expr.rs:9:1: 9:2
+        drop(_4) -> bb6;                 // scope 1 at $DIR/box_expr.rs:8:11: 8:12
     }
 
     bb6 (cleanup): {
-        drop(_2) -> bb7;                 // scope 0 at $DIR/box_expr.rs:7:24: 7:25
+        drop(_1) -> bb8;                 // scope 0 at $DIR/box_expr.rs:9:1: 9:2
     }
 
     bb7 (cleanup): {
+        drop(_2) -> bb8;                 // scope 0 at $DIR/box_expr.rs:7:24: 7:25
+    }
+
+    bb8 (cleanup): {
         resume;                          // scope 0 at $DIR/box_expr.rs:6:1: 9:2
     }
 }
index bbbd2bcf128b1e3abae333228f0ebf604718d635..7113c42b9c77f97088ae3ab70be33c53fdb48e96 100644 (file)
@@ -6,18 +6,21 @@ fn main() -> () {
     let mut _2: S;                       // in scope 0 at $DIR/issue-41110.rs:8:13: 8:14
     let mut _3: S;                       // in scope 0 at $DIR/issue-41110.rs:8:21: 8:27
     let mut _4: S;                       // in scope 0 at $DIR/issue-41110.rs:8:21: 8:22
+    let mut _5: bool;                    // in scope 0 at $DIR/issue-41110.rs:8:27: 8:28
     scope 1 {
         debug x => _1;                   // in scope 1 at $DIR/issue-41110.rs:8:9: 8:10
     }
 
     bb0: {
+        _5 = const false;                // scope 0 at $DIR/issue-41110.rs:8:9: 8:10
         StorageLive(_1);                 // scope 0 at $DIR/issue-41110.rs:8:9: 8:10
         StorageLive(_2);                 // scope 0 at $DIR/issue-41110.rs:8:13: 8:14
+        _5 = const true;                 // scope 0 at $DIR/issue-41110.rs:8:13: 8:14
         _2 = S;                          // scope 0 at $DIR/issue-41110.rs:8:13: 8:14
         StorageLive(_3);                 // scope 0 at $DIR/issue-41110.rs:8:21: 8:27
         StorageLive(_4);                 // scope 0 at $DIR/issue-41110.rs:8:21: 8:22
         _4 = S;                          // scope 0 at $DIR/issue-41110.rs:8:21: 8:22
-        _3 = S::id(move _4) -> [return: bb1, unwind: bb3]; // scope 0 at $DIR/issue-41110.rs:8:21: 8:27
+        _3 = S::id(move _4) -> [return: bb1, unwind: bb4]; // scope 0 at $DIR/issue-41110.rs:8:21: 8:27
                                          // mir::Constant
                                          // + span: $DIR/issue-41110.rs:8:23: 8:25
                                          // + literal: Const { ty: fn(S) -> S {S::id}, val: Value(Scalar(<ZST>)) }
@@ -25,7 +28,8 @@ fn main() -> () {
 
     bb1: {
         StorageDead(_4);                 // scope 0 at $DIR/issue-41110.rs:8:26: 8:27
-        _1 = S::other(move _2, move _3) -> bb2; // scope 0 at $DIR/issue-41110.rs:8:13: 8:28
+        _5 = const false;                // scope 0 at $DIR/issue-41110.rs:8:13: 8:28
+        _1 = S::other(move _2, move _3) -> [return: bb2, unwind: bb3]; // scope 0 at $DIR/issue-41110.rs:8:13: 8:28
                                          // mir::Constant
                                          // + span: $DIR/issue-41110.rs:8:15: 8:20
                                          // + literal: Const { ty: fn(S, S) {S::other}, val: Value(Scalar(<ZST>)) }
@@ -33,6 +37,7 @@ fn main() -> () {
 
     bb2: {
         StorageDead(_3);                 // scope 0 at $DIR/issue-41110.rs:8:27: 8:28
+        _5 = const false;                // scope 0 at $DIR/issue-41110.rs:8:27: 8:28
         StorageDead(_2);                 // scope 0 at $DIR/issue-41110.rs:8:27: 8:28
         _0 = const ();                   // scope 0 at $DIR/issue-41110.rs:7:11: 9:2
         StorageDead(_1);                 // scope 0 at $DIR/issue-41110.rs:9:1: 9:2
@@ -40,10 +45,26 @@ fn main() -> () {
     }
 
     bb3 (cleanup): {
-        drop(_2) -> bb4;                 // scope 0 at $DIR/issue-41110.rs:8:27: 8:28
+        goto -> bb5;                     // scope 0 at $DIR/issue-41110.rs:8:27: 8:28
     }
 
     bb4 (cleanup): {
+        goto -> bb5;                     // scope 0 at $DIR/issue-41110.rs:8:26: 8:27
+    }
+
+    bb5 (cleanup): {
+        goto -> bb8;                     // scope 0 at $DIR/issue-41110.rs:8:27: 8:28
+    }
+
+    bb6 (cleanup): {
         resume;                          // scope 0 at $DIR/issue-41110.rs:7:1: 9:2
     }
+
+    bb7 (cleanup): {
+        drop(_2) -> bb6;                 // scope 0 at $DIR/issue-41110.rs:8:27: 8:28
+    }
+
+    bb8 (cleanup): {
+        switchInt(_5) -> [false: bb6, otherwise: bb7]; // scope 0 at $DIR/issue-41110.rs:8:27: 8:28
+    }
 }
index be730402be981b1daba6059a5c06e33efca3ad43..c4e852ca3212a5d9eddf51c4476cfcfedf6335e3 100644 (file)
@@ -37,7 +37,7 @@ fn test() -> () {
         StorageLive(_5);                 // scope 2 at $DIR/issue-41110.rs:18:9: 18:10
         _6 = const false;                // scope 2 at $DIR/issue-41110.rs:18:9: 18:10
         _5 = move _1;                    // scope 2 at $DIR/issue-41110.rs:18:9: 18:10
-        goto -> bb11;                    // scope 2 at $DIR/issue-41110.rs:18:5: 18:6
+        goto -> bb12;                    // scope 2 at $DIR/issue-41110.rs:18:5: 18:6
     }
 
     bb2: {
@@ -47,7 +47,7 @@ fn test() -> () {
     bb3: {
         StorageDead(_5);                 // scope 2 at $DIR/issue-41110.rs:18:9: 18:10
         _0 = const ();                   // scope 0 at $DIR/issue-41110.rs:14:15: 19:2
-        drop(_2) -> [return: bb4, unwind: bb8]; // scope 1 at $DIR/issue-41110.rs:19:1: 19:2
+        drop(_2) -> [return: bb4, unwind: bb9]; // scope 1 at $DIR/issue-41110.rs:19:1: 19:2
     }
 
     bb4: {
@@ -62,36 +62,40 @@ fn test() -> () {
     }
 
     bb6 (cleanup): {
-        goto -> bb7;                     // scope 2 at $DIR/issue-41110.rs:18:9: 18:10
+        goto -> bb8;                     // scope 2 at $DIR/issue-41110.rs:18:9: 18:10
     }
 
     bb7 (cleanup): {
-        goto -> bb8;                     // scope 1 at $DIR/issue-41110.rs:19:1: 19:2
+        goto -> bb8;                     // scope 2 at $DIR/issue-41110.rs:17:11: 17:12
     }
 
     bb8 (cleanup): {
-        goto -> bb13;                    // scope 0 at $DIR/issue-41110.rs:19:1: 19:2
+        goto -> bb9;                     // scope 1 at $DIR/issue-41110.rs:19:1: 19:2
     }
 
     bb9 (cleanup): {
-        resume;                          // scope 0 at $DIR/issue-41110.rs:14:1: 19:2
+        goto -> bb14;                    // scope 0 at $DIR/issue-41110.rs:19:1: 19:2
     }
 
     bb10 (cleanup): {
+        resume;                          // scope 0 at $DIR/issue-41110.rs:14:1: 19:2
+    }
+
+    bb11 (cleanup): {
         _2 = move _5;                    // scope 2 at $DIR/issue-41110.rs:18:5: 18:6
         goto -> bb6;                     // scope 2 at $DIR/issue-41110.rs:18:5: 18:6
     }
 
-    bb11: {
+    bb12: {
         _2 = move _5;                    // scope 2 at $DIR/issue-41110.rs:18:5: 18:6
         goto -> bb2;                     // scope 2 at $DIR/issue-41110.rs:18:5: 18:6
     }
 
-    bb12 (cleanup): {
-        drop(_1) -> bb9;                 // scope 0 at $DIR/issue-41110.rs:19:1: 19:2
+    bb13 (cleanup): {
+        drop(_1) -> bb10;                // scope 0 at $DIR/issue-41110.rs:19:1: 19:2
     }
 
-    bb13 (cleanup): {
-        switchInt(_6) -> [false: bb9, otherwise: bb12]; // scope 0 at $DIR/issue-41110.rs:19:1: 19:2
+    bb14 (cleanup): {
+        switchInt(_6) -> [false: bb10, otherwise: bb13]; // scope 0 at $DIR/issue-41110.rs:19:1: 19:2
     }
 }
index 2f95931d2b2a1861a15aaea536cf8ceebde26462..bbb433dbe25c787203525dbe6561d0ff04946023 100644 (file)
@@ -28,7 +28,7 @@ fn main() -> () {
 
     bb1: {
         StorageDead(_3);                 // scope 0 at $DIR/no-spurious-drop-after-call.rs:9:33: 9:34
-        _1 = std::mem::drop::<String>(move _2) -> bb2; // scope 0 at $DIR/no-spurious-drop-after-call.rs:9:5: 9:35
+        _1 = std::mem::drop::<String>(move _2) -> [return: bb2, unwind: bb3]; // scope 0 at $DIR/no-spurious-drop-after-call.rs:9:5: 9:35
                                          // mir::Constant
                                          // + span: $DIR/no-spurious-drop-after-call.rs:9:5: 9:19
                                          // + literal: Const { ty: fn(std::string::String) {std::mem::drop::<std::string::String>}, val: Value(Scalar(<ZST>)) }
@@ -41,4 +41,12 @@ fn main() -> () {
         _0 = const ();                   // scope 0 at $DIR/no-spurious-drop-after-call.rs:8:11: 10:2
         return;                          // scope 0 at $DIR/no-spurious-drop-after-call.rs:10:2: 10:2
     }
+
+    bb3 (cleanup): {
+        drop(_2) -> bb4;                 // scope 0 at $DIR/no-spurious-drop-after-call.rs:9:34: 9:35
+    }
+
+    bb4 (cleanup): {
+        resume;                          // scope 0 at $DIR/no-spurious-drop-after-call.rs:8:1: 10:2
+    }
 }