From 9b566401068cb8450912f6ab48f3d0e60f5cb482 Mon Sep 17 00:00:00 2001 From: Ding Xiang Fei Date: Thu, 21 Jul 2022 00:35:12 +0800 Subject: [PATCH] break out scopes when let-else fails to match --- compiler/rustc_mir_build/src/build/block.rs | 1 + .../rustc_mir_build/src/build/matches/mod.rs | 86 ++++++++++--------- compiler/rustc_mir_build/src/build/scope.rs | 4 +- .../ui/let-else/let-else-temp-borrowck.rs | 26 ++++++ .../let-else/let-else-temporary-lifetime.rs | 35 ++++++++ 5 files changed, 110 insertions(+), 42 deletions(-) create mode 100644 src/test/ui/let-else/let-else-temp-borrowck.rs diff --git a/compiler/rustc_mir_build/src/build/block.rs b/compiler/rustc_mir_build/src/build/block.rs index cb8be51a08562..6875600129a8f 100644 --- a/compiler/rustc_mir_build/src/build/block.rs +++ b/compiler/rustc_mir_build/src/build/block.rs @@ -132,6 +132,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { initializer_span, else_block, visibility_scope, + *remainder_scope, remainder_span, pattern, ) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 7067a48b783ec..58b1564cc5d8c 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -2282,49 +2282,55 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { initializer_span: Span, else_block: &Block, visibility_scope: Option, + remainder_scope: region::Scope, remainder_span: Span, pattern: &Pat<'tcx>, ) -> BlockAnd<()> { - let scrutinee = unpack!(block = self.lower_scrutinee(block, init, initializer_span)); - let pat = Pat { ty: init.ty, span: else_block.span, kind: Box::new(PatKind::Wild) }; - let mut wildcard = Candidate::new(scrutinee.clone(), &pat, false); - self.declare_bindings( - visibility_scope, - remainder_span, - pattern, - ArmHasGuard(false), - Some((None, initializer_span)), - ); - let mut candidate = Candidate::new(scrutinee.clone(), pattern, false); - let fake_borrow_temps = self.lower_match_tree( - block, - initializer_span, - pattern.span, - false, - &mut [&mut candidate, &mut wildcard], - ); - // This block is for the matching case - let matching = self.bind_pattern( - self.source_info(pattern.span), - candidate, - None, - &fake_borrow_temps, - initializer_span, - None, - None, - None, - ); - // This block is for the failure case - let failure = self.bind_pattern( - self.source_info(else_block.span), - wildcard, - None, - &fake_borrow_temps, - initializer_span, - None, - None, - None, - ); + let (matching, failure) = self.in_if_then_scope(remainder_scope, |this| { + let scrutinee = unpack!(block = this.lower_scrutinee(block, init, initializer_span)); + let pat = Pat { ty: init.ty, span: else_block.span, kind: Box::new(PatKind::Wild) }; + let mut wildcard = Candidate::new(scrutinee.clone(), &pat, false); + this.declare_bindings( + visibility_scope, + remainder_span, + pattern, + ArmHasGuard(false), + Some((None, initializer_span)), + ); + let mut candidate = Candidate::new(scrutinee.clone(), pattern, false); + let fake_borrow_temps = this.lower_match_tree( + block, + initializer_span, + pattern.span, + false, + &mut [&mut candidate, &mut wildcard], + ); + // This block is for the matching case + let matching = this.bind_pattern( + this.source_info(pattern.span), + candidate, + None, + &fake_borrow_temps, + initializer_span, + None, + None, + None, + ); + // This block is for the failure case + let failure = this.bind_pattern( + this.source_info(else_block.span), + wildcard, + None, + &fake_borrow_temps, + initializer_span, + None, + None, + None, + ); + this.break_for_else(failure, remainder_scope, this.source_info(initializer_span)); + matching.unit() + }); + // This place is not really used because this destination place // should never be used to take values at the end of the failure // block. diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index b9fd8c50e6a04..b2fd9f25bdde7 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -690,7 +690,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } drops.add_entry(block, drop_idx); - // `build_drop_tree` doesn't have access to our source_info, so we + // `build_drop_trees` doesn't have access to our source_info, so we // create a dummy terminator now. `TerminatorKind::Resume` is used // because MIR type checking will panic if it hasn't been overwritten. self.cfg.terminate(block, source_info, TerminatorKind::Resume); @@ -722,7 +722,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } drops.add_entry(block, drop_idx); - // `build_drop_tree` doesn't have access to our source_info, so we + // `build_drop_trees` doesn't have access to our source_info, so we // create a dummy terminator now. `TerminatorKind::Resume` is used // because MIR type checking will panic if it hasn't been overwritten. self.cfg.terminate(block, source_info, TerminatorKind::Resume); diff --git a/src/test/ui/let-else/let-else-temp-borrowck.rs b/src/test/ui/let-else/let-else-temp-borrowck.rs new file mode 100644 index 0000000000000..3910d35e77676 --- /dev/null +++ b/src/test/ui/let-else/let-else-temp-borrowck.rs @@ -0,0 +1,26 @@ +// run-pass +// +// from issue #93951, where borrowck complained the temporary that `foo(&x)` was stored in was to +// be dropped sometime after `x` was. It then suggested adding a semicolon that was already there. + +#![feature(let_else)] +use std::fmt::Debug; + +fn foo<'a>(x: &'a str) -> Result { + Ok(x) +} + +fn let_else() { + let x = String::from("Hey"); + let Ok(_) = foo(&x) else { return }; +} + +fn if_let() { + let x = String::from("Hey"); + let _ = if let Ok(s) = foo(&x) { s } else { return }; +} + +fn main() { + let_else(); + if_let(); +} diff --git a/src/test/ui/let-else/let-else-temporary-lifetime.rs b/src/test/ui/let-else/let-else-temporary-lifetime.rs index 624c2ea37a70b..28c69ba1ce66f 100644 --- a/src/test/ui/let-else/let-else-temporary-lifetime.rs +++ b/src/test/ui/let-else/let-else-temporary-lifetime.rs @@ -1,6 +1,7 @@ // run-pass #![feature(let_else)] +use std::rc::Rc; use std::sync::atomic::{AtomicU8, Ordering}; static TRACKER: AtomicU8 = AtomicU8::new(0); @@ -22,4 +23,38 @@ fn main() { let 0 = Droppy::default().inner else { return }; assert_eq!(TRACKER.load(Ordering::Acquire), 1); println!("Should have dropped 👆"); + + { + // test let-else drops temps after statement + let rc = Rc::new(0); + let 0 = *rc.clone() else { unreachable!() }; + Rc::try_unwrap(rc).unwrap(); + } + { + let mut rc = Rc::new(0); + let mut i = 0; + loop { + if i > 3 { + break; + } + let 1 = *rc.clone() else { + if let Ok(v) = Rc::try_unwrap(rc) { + rc = Rc::new(v); + } else { + panic!() + } + i += 1; + continue + }; + } + } + { + // test let-else drops temps before else block + let rc = Rc::new(0); + let 1 = *rc.clone() else { + Rc::try_unwrap(rc).unwrap(); + return; + }; + unreachable!(); + } }