From 3bf7e843943424487253525bb1533439d2eea777 Mon Sep 17 00:00:00 2001 From: b-naber Date: Thu, 5 Jan 2023 14:27:37 +0100 Subject: [PATCH] introduce terminating scope in tail expressions of breakable scopes --- .../rustc_hir_analysis/src/check/region.rs | 7 + .../mir-opt/building/breakable_scope_drops.rs | 36 +++++ ...cope_drops.weird_temporary.built.after.mir | 134 ++++++++++++++++++ .../tail-expression-breakable-scope.rs | 24 ++++ .../tail-expression-breakable-scope.stderr | 15 ++ src/test/ui/try-block/dot-dot.rs | 20 +++ 6 files changed, 236 insertions(+) create mode 100644 src/test/mir-opt/building/breakable_scope_drops.rs create mode 100644 src/test/mir-opt/building/breakable_scope_drops.weird_temporary.built.after.mir create mode 100644 src/test/ui/borrowck/tail-expression-breakable-scope.rs create mode 100644 src/test/ui/borrowck/tail-expression-breakable-scope.stderr create mode 100644 src/test/ui/try-block/dot-dot.rs diff --git a/compiler/rustc_hir_analysis/src/check/region.rs b/compiler/rustc_hir_analysis/src/check/region.rs index b315ebad4686c..ecee49a22047c 100644 --- a/compiler/rustc_hir_analysis/src/check/region.rs +++ b/compiler/rustc_hir_analysis/src/check/region.rs @@ -166,6 +166,13 @@ fn resolve_block<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, blk: &'tcx h hir::StmtKind::Expr(..) | hir::StmtKind::Semi(..) => visitor.visit_stmt(statement), } } + + // We must introduce a terminating scope for tail expressions in breakable + // scopes, otherwise incorrect drops and unwind paths are created, see #104736. + if blk.targeted_by_break && let Some(expr) = blk.expr { + visitor.terminating_scopes.insert(expr.hir_id.local_id); + } + walk_list!(visitor, visit_expr, &blk.expr); } diff --git a/src/test/mir-opt/building/breakable_scope_drops.rs b/src/test/mir-opt/building/breakable_scope_drops.rs new file mode 100644 index 0000000000000..b6eff6eb9033a --- /dev/null +++ b/src/test/mir-opt/building/breakable_scope_drops.rs @@ -0,0 +1,36 @@ +// EMIT_MIR breakable_scope_drops.weird_temporary.built.after.mir + +struct A; +struct B; + +impl Drop for A { + fn drop(&mut self) {} +} +impl Drop for B { + fn drop(&mut self) {} +} + +#[inline(always)] +fn no_unwind() {} + + +fn weird_temporary(a: A, b: B, nothing: ((), (), ()), x: bool) -> ((), (), ()) { + let _temp = 'scope: { + ( + { + let _z = b; + if x { + break 'scope nothing; + } + }, + match { a } { + _ => (), + }, + no_unwind(), + ) + }; + + nothing +} + +fn main() {} diff --git a/src/test/mir-opt/building/breakable_scope_drops.weird_temporary.built.after.mir b/src/test/mir-opt/building/breakable_scope_drops.weird_temporary.built.after.mir new file mode 100644 index 0000000000000..6453e164e61b3 --- /dev/null +++ b/src/test/mir-opt/building/breakable_scope_drops.weird_temporary.built.after.mir @@ -0,0 +1,134 @@ +// MIR for `weird_temporary` after built + +fn weird_temporary(_1: A, _2: B, _3: ((), (), ()), _4: bool) -> ((), (), ()) { + debug a => _1; // in scope 0 at $DIR/breakable-scope-drops.rs:+0:20: +0:21 + debug b => _2; // in scope 0 at $DIR/breakable-scope-drops.rs:+0:26: +0:27 + debug nothing => _3; // in scope 0 at $DIR/breakable-scope-drops.rs:+0:32: +0:39 + debug x => _4; // in scope 0 at $DIR/breakable-scope-drops.rs:+0:55: +0:56 + let mut _0: ((), (), ()); // return place in scope 0 at $DIR/breakable-scope-drops.rs:+0:67: +0:79 + let _5: ((), (), ()); // in scope 0 at $DIR/breakable-scope-drops.rs:+1:9: +1:14 + let mut _6: (); // in scope 0 at $DIR/breakable-scope-drops.rs:+3:13: +8:14 + let _7: B; // in scope 0 at $DIR/breakable-scope-drops.rs:+4:21: +4:23 + let mut _8: bool; // in scope 0 at $DIR/breakable-scope-drops.rs:+5:20: +5:21 + let mut _9: !; // in scope 0 at $DIR/breakable-scope-drops.rs:+5:22: +7:18 + let mut _10: (); // in scope 0 at $DIR/breakable-scope-drops.rs:+9:13: +11:14 + let mut _11: A; // in scope 0 at $DIR/breakable-scope-drops.rs:+9:19: +9:24 + let mut _12: (); // in scope 0 at $DIR/breakable-scope-drops.rs:+12:13: +12:24 + scope 1 { + debug _temp => _5; // in scope 1 at $DIR/breakable-scope-drops.rs:+1:9: +1:14 + } + scope 2 { + debug _z => _7; // in scope 2 at $DIR/breakable-scope-drops.rs:+4:21: +4:23 + } + + bb0: { + StorageLive(_5); // scope 0 at $DIR/breakable-scope-drops.rs:+1:9: +1:14 + StorageLive(_6); // scope 0 at $DIR/breakable-scope-drops.rs:+3:13: +8:14 + StorageLive(_7); // scope 0 at $DIR/breakable-scope-drops.rs:+4:21: +4:23 + _7 = move _2; // scope 0 at $DIR/breakable-scope-drops.rs:+4:26: +4:27 + FakeRead(ForLet(None), _7); // scope 0 at $DIR/breakable-scope-drops.rs:+4:21: +4:23 + StorageLive(_8); // scope 2 at $DIR/breakable-scope-drops.rs:+5:20: +5:21 + _8 = _4; // scope 2 at $DIR/breakable-scope-drops.rs:+5:20: +5:21 + switchInt(move _8) -> [0: bb2, otherwise: bb1]; // scope 2 at $DIR/breakable-scope-drops.rs:+5:20: +5:21 + } + + bb1: { + _5 = _3; // scope 2 at $DIR/breakable-scope-drops.rs:+6:34: +6:41 + goto -> bb11; // scope 2 at $DIR/breakable-scope-drops.rs:+6:21: +6:41 + } + + bb2: { + goto -> bb5; // scope 2 at $DIR/breakable-scope-drops.rs:+5:20: +5:21 + } + + bb3: { + unreachable; // scope 2 at $DIR/breakable-scope-drops.rs:+5:22: +7:18 + } + + bb4: { + goto -> bb6; // scope 2 at $DIR/breakable-scope-drops.rs:+5:17: +7:18 + } + + bb5: { + _6 = const (); // scope 2 at $DIR/breakable-scope-drops.rs:+7:18: +7:18 + goto -> bb6; // scope 2 at $DIR/breakable-scope-drops.rs:+5:17: +7:18 + } + + bb6: { + StorageDead(_8); // scope 2 at $DIR/breakable-scope-drops.rs:+7:17: +7:18 + drop(_7) -> [return: bb7, unwind: bb17]; // scope 0 at $DIR/breakable-scope-drops.rs:+8:13: +8:14 + } + + bb7: { + StorageDead(_7); // scope 0 at $DIR/breakable-scope-drops.rs:+8:13: +8:14 + StorageLive(_10); // scope 0 at $DIR/breakable-scope-drops.rs:+9:13: +11:14 + StorageLive(_11); // scope 0 at $DIR/breakable-scope-drops.rs:+9:19: +9:24 + _11 = move _1; // scope 0 at $DIR/breakable-scope-drops.rs:+9:21: +9:22 + FakeRead(ForMatchedPlace(None), _11); // scope 0 at $DIR/breakable-scope-drops.rs:+9:19: +9:24 + _10 = (); // scope 0 at $DIR/breakable-scope-drops.rs:+10:22: +10:24 + goto -> bb8; // scope 0 at $DIR/breakable-scope-drops.rs:+10:22: +10:24 + } + + bb8: { + StorageLive(_12); // scope 0 at $DIR/breakable-scope-drops.rs:+12:13: +12:24 + _12 = no_unwind() -> [return: bb9, unwind: bb16]; // scope 0 at $DIR/breakable-scope-drops.rs:+12:13: +12:24 + // mir::Constant + // + span: $DIR/breakable-scope-drops.rs:29:13: 29:22 + // + literal: Const { ty: fn() {no_unwind}, val: Value() } + } + + bb9: { + _5 = (move _6, move _10, move _12); // scope 0 at $DIR/breakable-scope-drops.rs:+2:9: +13:10 + StorageDead(_12); // scope 0 at $DIR/breakable-scope-drops.rs:+13:9: +13:10 + drop(_11) -> [return: bb10, unwind: bb17]; // scope 0 at $DIR/breakable-scope-drops.rs:+13:9: +13:10 + } + + bb10: { + StorageDead(_11); // scope 0 at $DIR/breakable-scope-drops.rs:+13:9: +13:10 + StorageDead(_10); // scope 0 at $DIR/breakable-scope-drops.rs:+13:9: +13:10 + StorageDead(_6); // scope 0 at $DIR/breakable-scope-drops.rs:+13:9: +13:10 + goto -> bb13; // scope 0 at $DIR/breakable-scope-drops.rs:+1:17: +14:6 + } + + bb11: { + StorageDead(_8); // scope 2 at $DIR/breakable-scope-drops.rs:+7:17: +7:18 + drop(_7) -> [return: bb12, unwind: bb17]; // scope 0 at $DIR/breakable-scope-drops.rs:+8:13: +8:14 + } + + bb12: { + StorageDead(_7); // scope 0 at $DIR/breakable-scope-drops.rs:+8:13: +8:14 + StorageDead(_6); // scope 0 at $DIR/breakable-scope-drops.rs:+13:9: +13:10 + goto -> bb13; // scope 0 at $DIR/breakable-scope-drops.rs:+1:17: +14:6 + } + + bb13: { + FakeRead(ForLet(None), _5); // scope 0 at $DIR/breakable-scope-drops.rs:+1:9: +1:14 + _0 = _3; // scope 1 at $DIR/breakable-scope-drops.rs:+16:5: +16:12 + StorageDead(_5); // scope 0 at $DIR/breakable-scope-drops.rs:+17:1: +17:2 + drop(_2) -> [return: bb14, unwind: bb18]; // scope 0 at $DIR/breakable-scope-drops.rs:+17:1: +17:2 + } + + bb14: { + drop(_1) -> [return: bb15, unwind: bb19]; // scope 0 at $DIR/breakable-scope-drops.rs:+17:1: +17:2 + } + + bb15: { + return; // scope 0 at $DIR/breakable-scope-drops.rs:+17:2: +17:2 + } + + bb16 (cleanup): { + drop(_11) -> bb17; // scope 0 at $DIR/breakable-scope-drops.rs:+13:9: +13:10 + } + + bb17 (cleanup): { + drop(_2) -> bb18; // scope 0 at $DIR/breakable-scope-drops.rs:+17:1: +17:2 + } + + bb18 (cleanup): { + drop(_1) -> bb19; // scope 0 at $DIR/breakable-scope-drops.rs:+17:1: +17:2 + } + + bb19 (cleanup): { + resume; // scope 0 at $DIR/breakable-scope-drops.rs:+0:1: +17:2 + } +} diff --git a/src/test/ui/borrowck/tail-expression-breakable-scope.rs b/src/test/ui/borrowck/tail-expression-breakable-scope.rs new file mode 100644 index 0000000000000..71d6874875040 --- /dev/null +++ b/src/test/ui/borrowck/tail-expression-breakable-scope.rs @@ -0,0 +1,24 @@ +struct A; + +impl Drop for A { + fn drop(&mut self) {} +} + +fn takes_a_ref<'a>(_arg: &'a A) {} + +fn returns_a() -> A { + A +} + +fn weird_temporary<'a>(a: &'a A, x: bool) { + takes_a_ref('scope: { + if x { + break 'scope a; + } + + &returns_a() + //~^ ERROR temporary value dropped while borrowed + }); +} + +fn main() {} diff --git a/src/test/ui/borrowck/tail-expression-breakable-scope.stderr b/src/test/ui/borrowck/tail-expression-breakable-scope.stderr new file mode 100644 index 0000000000000..9c3f15bfa720c --- /dev/null +++ b/src/test/ui/borrowck/tail-expression-breakable-scope.stderr @@ -0,0 +1,15 @@ +error[E0716]: temporary value dropped while borrowed + --> $DIR/tail-expression-breakable-scope.rs:19:10 + | +LL | &returns_a() + | -^^^^^^^^^^- + | || | + | || temporary value is freed at the end of this statement + | |creates a temporary value which is freed while still in use + | borrow later used here + | + = note: consider using a `let` binding to create a longer lived value + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0716`. diff --git a/src/test/ui/try-block/dot-dot.rs b/src/test/ui/try-block/dot-dot.rs new file mode 100644 index 0000000000000..99131132f0eb2 --- /dev/null +++ b/src/test/ui/try-block/dot-dot.rs @@ -0,0 +1,20 @@ +// compile-flags: --crate-type=lib -Copt-level=3 -Zvalidate-mir --edition=2021 +// build-pass + +#![feature(try_blocks)] + +#[derive(Default, Debug)] +struct Response { + bookmarks: String, + continue_after: String, +} + +fn format_response(page: Result) -> Result { + try { + Response { + bookmarks: String::new(), + continue_after: page?, + ..Default::default() + } + } +}