]> git.lizzy.rs Git - rust.git/commitdiff
Drop function parameters in expected order
authorMatthew Jasper <mjjasper1@gmail.com>
Sun, 18 Nov 2018 14:49:24 +0000 (14:49 +0000)
committerMatthew Jasper <mjjasper1@gmail.com>
Fri, 30 Nov 2018 19:43:41 +0000 (19:43 +0000)
Given the function

fn foo((_x, _): (LogDrop, LogDrop), (_, _y): (LogDrop, LogDrop)) {}

Prior to 1.12 we dropped both `_x` and `_y` before the rest of their
respective parameters, since then we dropped `_x` and `_y` after. The
original order appears to be the correct order, as the value created
later is dropped first, so we revert to that order and add a test for
it.

src/librustc_mir/build/mod.rs
src/test/run-pass/binding/fn-arg-incomplete-pattern-drop-order.rs [new file with mode: 0644]

index d95a74be77696018fffe42c381b30c9885af4e69..bc12c262716e6d294eaeacf70420a3a281d1f077 100644 (file)
@@ -915,6 +915,13 @@ fn args_and_body(&mut self,
             let place = Place::Local(local);
             let &ArgInfo(ty, opt_ty_info, pattern, ref self_binding) = arg_info;
 
+            // Make sure we drop (parts of) the argument even when not matched on.
+            self.schedule_drop(
+                pattern.as_ref().map_or(ast_body.span, |pat| pat.span),
+                argument_scope, &place, ty,
+                DropKind::Value { cached_block: CachedBlock::default() },
+            );
+
             if let Some(pattern) = pattern {
                 let pattern = self.hir.pattern_from_hir(pattern);
                 let span = pattern.span;
@@ -946,13 +953,6 @@ fn args_and_body(&mut self,
                     }
                 }
             }
-
-            // Make sure we drop (parts of) the argument even when not matched on.
-            self.schedule_drop(
-                pattern.as_ref().map_or(ast_body.span, |pat| pat.span),
-                argument_scope, &place, ty,
-                DropKind::Value { cached_block: CachedBlock::default() },
-            );
         }
 
         // Enter the argument pattern bindings source scope, if it exists.
diff --git a/src/test/run-pass/binding/fn-arg-incomplete-pattern-drop-order.rs b/src/test/run-pass/binding/fn-arg-incomplete-pattern-drop-order.rs
new file mode 100644 (file)
index 0000000..4d5a6fb
--- /dev/null
@@ -0,0 +1,68 @@
+// Check that partially moved from function parameters are dropped after the
+// named bindings that move from them.
+
+// ignore-wasm32-bare compiled with panic=abort by default
+
+use std::{panic, cell::RefCell};
+
+struct LogDrop<'a>(i32, Context<'a>);
+
+#[derive(Copy, Clone)]
+struct Context<'a> {
+    panic_on: i32,
+    drops: &'a RefCell<Vec<i32>>,
+}
+
+impl<'a> Context<'a> {
+    fn record_drop(self, index: i32) {
+        self.drops.borrow_mut().push(index);
+        if index == self.panic_on {
+            panic!();
+        }
+    }
+}
+
+impl<'a> Drop for LogDrop<'a> {
+    fn drop(&mut self) {
+        self.1.record_drop(self.0);
+    }
+}
+
+fn bindings_in_params((_x, _): (LogDrop, LogDrop), (_, _y): (LogDrop, LogDrop)) {}
+fn bindings_with_let(a: (LogDrop, LogDrop), b: (LogDrop, LogDrop)) {
+    // Drop order in foo is the same as the following bindings.
+    // _temp2 is declared after _x to avoid a difference between `_: T` and
+    // `x: T` in function parameters.
+    let _temp1 = a;
+    let (_x, _) = _temp1;
+
+    let _temp2 = b;
+    let (_, _y) = _temp2;
+}
+
+fn test_drop_order(panic_on: i32, fun: fn((LogDrop, LogDrop), (LogDrop, LogDrop))) {
+    let context = Context {
+        panic_on,
+        drops: &RefCell::new(Vec::new()),
+    };
+    let one = LogDrop(1, context);
+    let two = LogDrop(2, context);
+    let three = LogDrop(3, context);
+    let four = LogDrop(4, context);
+
+    let res = panic::catch_unwind(panic::AssertUnwindSafe(|| {
+        fun((three, four), (two, one));
+    }));
+    if panic_on == 0 {
+        assert!(res.is_ok(), "should not have panicked");
+    } else {
+        assert!(res.is_err(), "should have panicked");
+    }
+    assert_eq!(*context.drops.borrow(), [1, 2, 3, 4], "incorrect drop order");
+}
+
+fn main() {
+    (0..=4).for_each(|i| test_drop_order(i, bindings_in_params));
+    (0..=4).for_each(|i| test_drop_order(i, bindings_with_let));
+    (0..=4).for_each(|i| test_drop_order(i, |(_x, _), (_, _y)| {}));
+}