about summary refs log tree commit diff
diff options
context:
space:
mode:
authorDylan DPC <99973273+Dylan-DPC@users.noreply.github.com>2022-10-19 14:05:52 +0530
committerGitHub <noreply@github.com>2022-10-19 14:05:52 +0530
commit48c5e0c262c4b454ec1c4528b5a9d81d3fc6bde6 (patch)
treee08004f9f2b1021ca95997f0ce432656f06ddf5f
parent5c2c476ad17b7cf4c883dedd3802b5b8a9d1b8f3 (diff)
parent3041bc9e71355eea71a5070b7939439fd3cf7e01 (diff)
downloadrust-48c5e0c262c4b454ec1c4528b5a9d81d3fc6bde6.tar.gz
rust-48c5e0c262c4b454ec1c4528b5a9d81d3fc6bde6.zip
Rollup merge of #103034 - nathanwhit:let-chains-rhs-temporaries, r=wesleywiser
Let expressions on RHS shouldn't be terminating scopes

Fixes #100276.

Before this PR, we were unconditionally marking the RHS of short-circuiting binary expressions as a terminating scope.

In the case of a let chain where the `let` expression was on the RHS, this meant that temporaries within the `let` expr would only live until the end of the expression. Since this only affected the RHS, this led to surprising behavior ([example](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=d1b0a5d1f01882f9c89c2194a75eb19f)).

After this PR, we only mark the RHS as a terminating scope if it is not a `let` expression.
-rw-r--r--compiler/rustc_hir_analysis/src/check/region.rs8
-rw-r--r--src/test/ui/drop/drop_order.rs4
-rw-r--r--src/test/ui/drop/issue-100276.rs12
-rw-r--r--src/test/ui/mir/mir_let_chains_drop_order.rs8
4 files changed, 24 insertions, 8 deletions
diff --git a/compiler/rustc_hir_analysis/src/check/region.rs b/compiler/rustc_hir_analysis/src/check/region.rs
index b89db79bef8..ff32329e431 100644
--- a/compiler/rustc_hir_analysis/src/check/region.rs
+++ b/compiler/rustc_hir_analysis/src/check/region.rs
@@ -252,9 +252,13 @@ fn resolve_expr<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, expr: &'tcx h
             ) => {
                 // For shortcircuiting operators, mark the RHS as a terminating
                 // scope since it only executes conditionally.
-                terminating(r.hir_id.local_id);
-            }
 
+                // `Let` expressions (in a let-chain) shouldn't be terminating, as their temporaries
+                // should live beyond the immediate expression
+                if !matches!(r.kind, hir::ExprKind::Let(_)) {
+                    terminating(r.hir_id.local_id);
+                }
+            }
             hir::ExprKind::If(_, ref then, Some(ref otherwise)) => {
                 terminating(then.hir_id.local_id);
                 terminating(otherwise.hir_id.local_id);
diff --git a/src/test/ui/drop/drop_order.rs b/src/test/ui/drop/drop_order.rs
index ba1ac53aa7c..42385216ae7 100644
--- a/src/test/ui/drop/drop_order.rs
+++ b/src/test/ui/drop/drop_order.rs
@@ -129,10 +129,10 @@ impl DropOrderCollector {
         // take the "else" branch
         if self.option_loud_drop(6).is_some() // 2
             && self.option_loud_drop(5).is_some() // 1
-            && let None = self.option_loud_drop(7) { // 3
+            && let None = self.option_loud_drop(8) { // 4
             unreachable!();
         } else {
-            self.print(8); // 4
+            self.print(7); // 3
         }
 
         // let exprs interspersed
diff --git a/src/test/ui/drop/issue-100276.rs b/src/test/ui/drop/issue-100276.rs
new file mode 100644
index 00000000000..6401a8d1481
--- /dev/null
+++ b/src/test/ui/drop/issue-100276.rs
@@ -0,0 +1,12 @@
+// check-pass
+// compile-flags: -Z validate-mir
+#![feature(let_chains)]
+
+fn let_chains(entry: std::io::Result<std::fs::DirEntry>) {
+    if let Ok(entry) = entry
+        && let Some(s) = entry.file_name().to_str()
+        && s.contains("")
+    {}
+}
+
+fn main() {}
diff --git a/src/test/ui/mir/mir_let_chains_drop_order.rs b/src/test/ui/mir/mir_let_chains_drop_order.rs
index 536a84a352a..6471553e93f 100644
--- a/src/test/ui/mir/mir_let_chains_drop_order.rs
+++ b/src/test/ui/mir/mir_let_chains_drop_order.rs
@@ -12,7 +12,7 @@ use std::panic;
 pub struct DropLogger<'a, T> {
     extra: T,
     id: usize,
-    log: &'a panic::AssertUnwindSafe<RefCell<Vec<usize>>>
+    log: &'a panic::AssertUnwindSafe<RefCell<Vec<usize>>>,
 }
 
 impl<'a, T> Drop for DropLogger<'a, T> {
@@ -55,9 +55,9 @@ fn main() {
             else {
                 // 10 is not constructed
                 d(10, None)
-            }
+            },
         );
-        assert_eq!(get(), vec![3, 8, 7, 1, 2]);
+        assert_eq!(get(), vec![8, 7, 1, 3, 2]);
     }
     assert_eq!(get(), vec![0, 4, 6, 9, 5]);
 
@@ -89,5 +89,5 @@ fn main() {
             panic::panic_any(InjectedFailure)
         );
     });
-    assert_eq!(get(), vec![14, 19, 20, 17, 15, 11, 18, 16, 12, 13]);
+    assert_eq!(get(), vec![20, 17, 15, 11, 19, 18, 16, 12, 14, 13]);
 }