Auto merge of #145342 - dianne:if-let-super-let, r=<try>

fix drop scope for `super let` bindings within `if let`

Author: rust-bors[bot]

compiler/rustc_hir_analysis/src/check/region.rs

@@ -490,12 +490,8 @@ fn resolve_local<'tcx>(
             //
             // Iterate up to the enclosing destruction scope to find the same scope that will also
             // be used for the result of the block itself.
-            while let Some(s) = visitor.cx.var_parent {
-                let parent = visitor.scope_tree.parent_map.get(&s).cloned();
-                if let Some(Scope { data: ScopeData::Destruction, .. }) = parent {
-                    break;
-                }
-                visitor.cx.var_parent = parent;
+            if let Some(inner_scope) = visitor.cx.var_parent {
+                (visitor.cx.var_parent, _) = visitor.scope_tree.default_temporary_scope(inner_scope)
             }
         }
     }

compiler/rustc_middle/src/middle/region.rs

@@ -299,4 +299,43 @@ impl ScopeTree {
 
         true
     }
+
+    /// Returns the scope of non-lifetime-extended temporaries within a given scope, as well as
+    /// whether we've recorded a potential backwards-incompatible change to lint on.
+    /// Returns `None` when no enclosing temporary scope is found, such as for static items.
+    pub fn default_temporary_scope(&self, inner: Scope) -> (Option<Scope>, Option<Scope>) {
+        let mut id = inner;
+        let mut backwards_incompatible = None;
+
+        while let Some(&p) = self.parent_map.get(&id) {
+            match p.data {
+                ScopeData::Destruction => {
+                    debug!("temporary_scope({inner:?}) = {id:?} [enclosing]");
+                    return (Some(id), backwards_incompatible);
+                }
+                ScopeData::IfThenRescope | ScopeData::MatchGuard => {
+                    debug!("temporary_scope({inner:?}) = {p:?} [enclosing]");
+                    return (Some(p), backwards_incompatible);
+                }
+                ScopeData::Node
+                | ScopeData::CallSite
+                | ScopeData::Arguments
+                | ScopeData::IfThen
+                | ScopeData::Remainder(_) => {
+                    // If we haven't already passed through a backwards-incompatible node,
+                    // then check if we are passing through one now and record it if so.
+                    // This is for now only working for cases where a temporary lifetime is
+                    // *shortened*.
+                    if backwards_incompatible.is_none() {
+                        backwards_incompatible =
+                            self.backwards_incompatible_scope.get(&p.local_id).copied();
+                    }
+                    id = p
+                }
+            }
+        }
+
+        debug!("temporary_scope({inner:?}) = None");
+        (None, backwards_incompatible)
+    }
 }

compiler/rustc_middle/src/ty/rvalue_scopes.rs

@@ -35,41 +35,8 @@ impl RvalueScopes {
         // if there's one. Static items, for instance, won't
         // have an enclosing scope, hence no scope will be
         // returned.
-        let mut id = Scope { local_id: expr_id, data: ScopeData::Node };
-        let mut backwards_incompatible = None;
-
-        while let Some(&p) = region_scope_tree.parent_map.get(&id) {
-            match p.data {
-                ScopeData::Destruction => {
-                    debug!("temporary_scope({expr_id:?}) = {id:?} [enclosing]");
-                    return (Some(id), backwards_incompatible);
-                }
-                ScopeData::IfThenRescope | ScopeData::MatchGuard => {
-                    debug!("temporary_scope({expr_id:?}) = {p:?} [enclosing]");
-                    return (Some(p), backwards_incompatible);
-                }
-                ScopeData::Node
-                | ScopeData::CallSite
-                | ScopeData::Arguments
-                | ScopeData::IfThen
-                | ScopeData::Remainder(_) => {
-                    // If we haven't already passed through a backwards-incompatible node,
-                    // then check if we are passing through one now and record it if so.
-                    // This is for now only working for cases where a temporary lifetime is
-                    // *shortened*.
-                    if backwards_incompatible.is_none() {
-                        backwards_incompatible = region_scope_tree
-                            .backwards_incompatible_scope
-                            .get(&p.local_id)
-                            .copied();
-                    }
-                    id = p
-                }
-            }
-        }
-
-        debug!("temporary_scope({expr_id:?}) = None");
-        (None, backwards_incompatible)
+        region_scope_tree
+            .default_temporary_scope(Scope { local_id: expr_id, data: ScopeData::Node })
     }
 
     /// Make an association between a sub-expression and an extended lifetime

tests/ui/drop/if-let-super-let.rs

@@ -0,0 +1,112 @@
+//! Test for #145328: ensure the lifetime of a `super let` binding within an `if let` scrutinee is
+//! at most the scope of the `if` condition's temporaries. Additionally, test `pin!` since it's
+//! implemented in terms of `super let` and exposes this behavior.
+//@ run-pass
+//@ revisions: e2021 e2024
+//@ [e2021] edition: 2021
+//@ [e2024] edition: 2024
+
+#![feature(if_let_guard)]
+#![feature(super_let)]
+#![expect(irrefutable_let_patterns)]
+
+use std::cell::RefCell;
+use std::pin::pin;
+
+fn main() {
+    // The `super let` bindings here should have the same scope as `if let` temporaries.
+    // In Rust 2021, this means it lives past the end of the `if` expression.
+    // In Rust 2024, this means it lives to the end of the `if`'s success block.
+    assert_drop_order(0..=2, |o| {
+        #[cfg(e2021)]
+        (
+            if let _ = { super let _x = o.log(2); } { o.push(0) },
+            o.push(1),
+        );
+        #[cfg(e2024)]
+        (
+            if let _ = { super let _x = o.log(1); } { o.push(0) },
+            o.push(2),
+        );
+    });
+    assert_drop_order(0..=2, |o| {
+        #[cfg(e2021)]
+        (
+            if let true = { super let _x = o.log(2); false } {} else { o.push(0) },
+            o.push(1),
+        );
+        #[cfg(e2024)]
+        (
+            if let true = { super let _x = o.log(0); false } {} else { o.push(1) },
+            o.push(2),
+        );
+    });
+
+    // `pin!` should behave likewise.
+    assert_drop_order(0..=2, |o| {
+        #[cfg(e2021)] (if let _ = pin!(o.log(2)) { o.push(0) }, o.push(1));
+        #[cfg(e2024)] (if let _ = pin!(o.log(1)) { o.push(0) }, o.push(2));
+    });
+    assert_drop_order(0..=2, |o| {
+        #[cfg(e2021)]
+        (
+            if let None = Some(pin!(o.log(2))) {} else { o.push(0) },
+            o.push(1),
+        );
+        #[cfg(e2024)]
+        (
+            if let None = Some(pin!(o.log(0))) {} else { o.push(1) },
+            o.push(2),
+        );
+    });
+
+    // `super let` bindings' scope should also be consistent with `if let` temporaries in guards.
+    // Here, that means the `super let` binding in the second guard condition operand should be
+    // dropped before the first operand's temporary. This is consistent across Editions.
+    assert_drop_order(0..=1, |o| {
+        match () {
+            _ if let _ = o.log(1)
+                && let _ = { super let _x = o.log(0); } => {}
+            _ => unreachable!(),
+        }
+    });
+    assert_drop_order(0..=1, |o| {
+        match () {
+            _ if let _ = o.log(1)
+                && let _ = pin!(o.log(0)) => {}
+            _ => unreachable!(),
+        }
+    });
+}
+
+// # Test scaffolding...
+
+struct DropOrder(RefCell<Vec<u64>>);
+struct LogDrop<'o>(&'o DropOrder, u64);
+
+impl DropOrder {
+    fn log(&self, n: u64) -> LogDrop<'_> {
+        LogDrop(self, n)
+    }
+    fn push(&self, n: u64) {
+        self.0.borrow_mut().push(n);
+    }
+}
+
+impl<'o> Drop for LogDrop<'o> {
+    fn drop(&mut self) {
+        self.0.push(self.1);
+    }
+}
+
+#[track_caller]
+fn assert_drop_order(
+    ex: impl IntoIterator<Item = u64>,
+    f: impl Fn(&DropOrder),
+) {
+    let order = DropOrder(RefCell::new(Vec::new()));
+    f(&order);
+    let order = order.0.into_inner();
+    let expected: Vec<u64> = ex.into_iter().collect();
+    assert_eq!(order, expected);
+}