about summary refs log tree commit diff
diff options
context:
space:
mode:
authorYuki Okushi <jtitor@2k36.org>2022-07-30 07:39:49 +0900
committerGitHub <noreply@github.com>2022-07-30 07:39:49 +0900
commit955091be8fded08338cb2ad8f0f2eb50e0a3df47 (patch)
tree49f04f071fec9c88308dbca02d05f491ad436246
parent36ab4ec2dc1795ed1d700d80437963c049ed551a (diff)
parent60be2de8b7b8a1c4eee7e065b8cef38ea629a6a3 (diff)
downloadrust-955091be8fded08338cb2ad8f0f2eb50e0a3df47.tar.gz
rust-955091be8fded08338cb2ad8f0f2eb50e0a3df47.zip
Rollup merge of #99518 - dingxiangfei2009:let-else-additional-tests, r=oli-obk
Let-else: break out scopes when a let-else pattern fails to match

This PR will commit to a new behavior so that values from initializer expressions are dropped earlier when a let-else pattern fails to match.

Fix #98672.
Close #93951.
cc `@camsteffen` `@est31`
-rw-r--r--compiler/rustc_mir_build/src/build/block.rs1
-rw-r--r--compiler/rustc_mir_build/src/build/matches/mod.rs86
-rw-r--r--compiler/rustc_mir_build/src/build/scope.rs4
-rw-r--r--src/test/ui/let-else/let-else-temp-borrowck.rs26
-rw-r--r--src/test/ui/let-else/let-else-temporary-lifetime.rs63
5 files changed, 138 insertions, 42 deletions
diff --git a/compiler/rustc_mir_build/src/build/block.rs b/compiler/rustc_mir_build/src/build/block.rs
index cb8be51a085..6875600129a 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 7067a48b783..58b1564cc5d 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<SourceScope>,
+        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 b9fd8c50e6a..b2fd9f25bdd 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 00000000000..3910d35e776
--- /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<impl Debug + 'a, ()> {
+    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 624c2ea37a7..9c86901b97f 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,8 @@
 // run-pass
 #![feature(let_else)]
 
+use std::fmt::Display;
+use std::rc::Rc;
 use std::sync::atomic::{AtomicU8, Ordering};
 
 static TRACKER: AtomicU8 = AtomicU8::new(0);
@@ -17,9 +19,70 @@ impl Drop for Droppy {
     }
 }
 
+fn foo<'a>(x: &'a str) -> Result<impl Display + 'a, ()> {
+    Ok(x)
+}
+
 fn main() {
     assert_eq!(TRACKER.load(Ordering::Acquire), 0);
     let 0 = Droppy::default().inner else { return };
     assert_eq!(TRACKER.load(Ordering::Acquire), 1);
     println!("Should have dropped 👆");
+
+    {
+        // cf. https://github.com/rust-lang/rust/pull/99518#issuecomment-1191520030
+        struct Foo<'a>(&'a mut u32);
+
+        impl<'a> Drop for Foo<'a> {
+            fn drop(&mut self) {
+                *self.0 = 0;
+            }
+        }
+        let mut foo = 0;
+        let Foo(0) = Foo(&mut foo) else {
+            *&mut foo = 1;
+            todo!()
+        };
+    }
+    {
+        let x = String::from("Hey");
+
+        let Ok(s) = foo(&x) else { panic!() };
+        assert_eq!(s.to_string(), x);
+    }
+    {
+        // 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
+        // NOTE: this test has to be the last block in the `main`
+        // body.
+        let rc = Rc::new(0);
+        let 1 = *rc.clone() else {
+            Rc::try_unwrap(rc).unwrap();
+            return;
+        };
+        unreachable!();
+    }
 }