From 914515f8090be23e7ae0acc29cdccf6360d1a03d Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sun, 18 Nov 2018 14:49:24 +0000 Subject: [PATCH] Drop function parameters in expected order 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 | 14 ++-- .../fn-arg-incomplete-pattern-drop-order.rs | 68 +++++++++++++++++++ 2 files changed, 75 insertions(+), 7 deletions(-) create mode 100644 src/test/run-pass/binding/fn-arg-incomplete-pattern-drop-order.rs diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index d95a74be77696..bc12c262716e6 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -915,6 +915,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; @@ -946,13 +953,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. 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 index 0000000000000..4d5a6fbba2857 --- /dev/null +++ b/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>, +} + +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)| {})); +}