Rollup merge of rust-lang#56044 - matthewjasper:function-param-drop-order, r=cramertj

Drop partially bound function parameters in the expected order

Given the function

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

Prior to 1.12.0 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.

While this is technically a breaking change, I can't work out how
anyone could be relying on this without making their code very
brittle. If this is considered to be too likely to break real world code
then I can revert the change and change the test to check for the
current order.

Author: Centril

src/librustc_mir/build/mod.rs

@@ -910,6 +910,13 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
             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;
@@ -941,13 +948,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                     }
                 }
             }
-
-            // 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.

src/test/run-pass/binding/fn-arg-incomplete-pattern-drop-order.rs

@@ -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)| {}));
+}