about summary refs log tree commit diff
path: root/src/test/ui
diff options
context:
space:
mode:
authorDylan DPC <99973273+Dylan-DPC@users.noreply.github.com>2022-10-15 15:45:32 +0530
committerGitHub <noreply@github.com>2022-10-15 15:45:32 +0530
commitb79ad57ad7dae81e27fb37b8c703dc4e4acdb7a7 (patch)
tree4fa9d32dfad33dd1b35e15ce4dc96a28b3aa67c1 /src/test/ui
parent59e0af68ab42c2c0b98b7e943ecfe01a36d90a94 (diff)
parent4e1c09dcd64507c03ded02844c1e5022031b9e38 (diff)
downloadrust-b79ad57ad7dae81e27fb37b8c703dc4e4acdb7a7.tar.gz
rust-b79ad57ad7dae81e27fb37b8c703dc4e4acdb7a7.zip
Rollup merge of #102998 - nathanwhit:let-chains-drop-order, r=eholk
Drop temporaries created in a condition, even if it's a let chain

Fixes #100513.

During the lowering from AST to HIR we wrap expressions acting as conditions in a `DropTemps` expression so that any temporaries created in the condition are dropped after the condition is executed. Effectively this means we transform

```rust
if Some(1).is_some() { .. }
```

into (roughly)

```rust
if { let _t = Some(1).is_some(); _t } { .. }
```

so that if we create any temporaries, they're lifted into the new scope surrounding the condition, so for example something along the lines of

```rust
if { let temp = Some(1); let _t = temp.is_some(); _t }.
```

Before this PR, if the condition contained any let expressions we would not introduce that new scope, instead leaving the condition alone. This meant that in a let-chain like

```rust
if get_drop("first").is_some() && let None = get_drop("last") {
        println!("second");
} else { .. }
```

the temporary created for `get_drop("first")` would be lifted into the _surrounding block_, which caused it to be dropped after the execution of the entire `if` expression.

After this PR, we wrap everything but the `let` expression in terminating scopes. The upside to this solution is that it's minimally invasive, but the downside is that in the worst case, an expression with `let` exprs interspersed like

```rust
if get_drop("first").is_some()
    && let Some(_a) = get_drop("fifth")
    && get_drop("second").is_some()
    && let Some(_b) = get_drop("fourth") { .. }
```

gets _multiple_ new scopes, roughly

```rust
if { let _t = get_drop("first").is_some(); _t }
    && let Some(_a) = get_drop("fifth")
    && { let _t = get_drop("second").is_some(); _t }
    && let Some(_b) = get_drop("fourth") { .. }
```

so instead of all of the temporaries being dropped at the end of the entire condition, they will be dropped right after they're evaluated (before the subsequent `let` expr). So while I'd say the drop behavior around let-chains is _less_ surprising after this PR, it still might not exactly match what people might expect.

For tests, I've just extended the drop order tests added in #100526. I'm not sure if that's the best way to go about it, though, so suggestions are welcome.
Diffstat (limited to 'src/test/ui')
-rw-r--r--src/test/ui/drop/drop_order.rs64
1 files changed, 64 insertions, 0 deletions
diff --git a/src/test/ui/drop/drop_order.rs b/src/test/ui/drop/drop_order.rs
index e42150dcc09..ba1ac53aa7c 100644
--- a/src/test/ui/drop/drop_order.rs
+++ b/src/test/ui/drop/drop_order.rs
@@ -1,4 +1,6 @@
 // run-pass
+// compile-flags: -Z validate-mir
+#![feature(let_chains)]
 
 use std::cell::RefCell;
 use std::convert::TryInto;
@@ -116,6 +118,58 @@ impl DropOrderCollector {
         }
     }
 
+    fn let_chain(&self) {
+        // take the "then" branch
+        if self.option_loud_drop(2).is_some() // 2
+            && self.option_loud_drop(1).is_some() // 1
+            && let Some(_d) = self.option_loud_drop(4) { // 4
+            self.print(3); // 3
+        }
+
+        // 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
+            unreachable!();
+        } else {
+            self.print(8); // 4
+        }
+
+        // let exprs interspersed
+        if self.option_loud_drop(9).is_some() // 1
+            && let Some(_d) = self.option_loud_drop(13) // 5
+            && self.option_loud_drop(10).is_some() // 2
+            && let Some(_e) = self.option_loud_drop(12) { // 4
+            self.print(11); // 3
+        }
+
+        // let exprs first
+        if let Some(_d) = self.option_loud_drop(18) // 5
+            && let Some(_e) = self.option_loud_drop(17) // 4
+            && self.option_loud_drop(14).is_some() // 1
+            && self.option_loud_drop(15).is_some() { // 2
+                self.print(16); // 3
+            }
+
+        // let exprs last
+        if self.option_loud_drop(20).is_some() // 2
+            && self.option_loud_drop(19).is_some() // 1
+            && let Some(_d) = self.option_loud_drop(23) // 5
+            && let Some(_e) = self.option_loud_drop(22) { // 4
+                self.print(21); // 3
+        }
+    }
+
+    fn while_(&self) {
+        let mut v = self.option_loud_drop(4);
+        while let Some(_d) = v
+            && self.option_loud_drop(1).is_some()
+            && self.option_loud_drop(2).is_some() {
+            self.print(3);
+            v = None;
+        }
+    }
+
     fn assert_sorted(self) {
         assert!(
             self.0
@@ -142,4 +196,14 @@ fn main() {
     let collector = DropOrderCollector::default();
     collector.match_();
     collector.assert_sorted();
+
+    println!("-- let chain --");
+    let collector = DropOrderCollector::default();
+    collector.let_chain();
+    collector.assert_sorted();
+
+    println!("-- while --");
+    let collector = DropOrderCollector::default();
+    collector.while_();
+    collector.assert_sorted();
 }