temporary lifetime extension for block tail expressions

compiler/rustc_hir_analysis/src/check/region.rs

@@ -8,7 +8,7 @@
 
 use std::mem;
 
-use rustc_data_structures::fx::FxHashMap;
+use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_hir as hir;
 use rustc_hir::def::{CtorKind, DefKind, Res};
 use rustc_hir::def_id::DefId;
@@ -38,6 +38,14 @@ struct ScopeResolutionVisitor<'tcx> {
 
     cx: Context,
 
+    /// Tracks [extending] block expressions. This is used in performing lifetime extension on block
+    /// tail expressions: if we've already extended the temporary scopes of extending borrows within
+    /// a block's tail when checking a parent `let` statement or block, we don't want to re-extend
+    /// them to be shorter when checking the block itself.
+    ///
+    /// [extending]: https://doc.rust-lang.org/nightly/reference/destructors.html#extending-based-on-expressions
+    extended_blocks: FxHashSet<hir::ItemLocalId>,
+
     extended_super_lets: FxHashMap<hir::ItemLocalId, Option<Scope>>,
 }
 
@@ -160,6 +168,20 @@ fn resolve_block<'tcx>(
                     .backwards_incompatible_scope
                     .insert(local_id, Scope { local_id, data: ScopeData::Node });
             }
+            // If we haven't already checked for temporary lifetime extension due to a parent `let`
+            // statement initializer or block, do so. This, e.g., allows `temp()` in `{ &temp() }`
+            // to outlive the block even when the block itself is not in a `let` statement
+            // initializer. The same rules for `let` are used here, so non-extending borrows are
+            // unaffected: `{ f(&temp()) }` drops `temp()` at the end of the block.
+            // NB: This should be checked even if the block is from Rust 2021 or before. Macro
+            // expansion can result in nested blocks from different editions, and we always want to
+            // propagate the outermost extending lifetime to the innermost extending expressions.
+            if !visitor.extended_blocks.contains(&blk.hir_id.local_id) {
+                let blk_result_scope = prev_cx.parent.and_then(|blk_parent| {
+                    visitor.scope_tree.default_temporary_scope(blk_parent).0
+                });
+                record_rvalue_scope_if_borrow_expr(visitor, tail_expr, blk_result_scope);
+            }
             resolve_expr(visitor, tail_expr, terminating);
         }
     }
@@ -467,9 +489,10 @@ fn resolve_local<'tcx>(
     // A, but the inner rvalues `a()` and `b()` have an extended lifetime
     // due to rule C.
 
+    let mut extend_initializer = true;
     if let_kind == LetKind::Super {
         if let Some(scope) = visitor.extended_super_lets.remove(&pat.unwrap().hir_id.local_id) {
-            // This expression was lifetime-extended by a parent let binding. E.g.
+            // This expression was lifetime-extended by a parent let binding or block. E.g.
             //
             //     let a = {
             //         super let b = temp();
@@ -482,25 +505,35 @@ fn resolve_local<'tcx>(
             // `super let` to its own var_scope. We use that scope.
             visitor.cx.var_parent = scope;
         } else {
-            // This `super let` is not subject to lifetime extension from a parent let binding. E.g.
+            // This `super let` is not subject to lifetime extension from a parent let binding or
+            // block. E.g.
             //
             //     identity({ super let x = temp(); &x }).method();
             //
             // (Which needs to behave exactly as: identity(&temp()).method();)
             //
             // 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)
             }
+            // Don't apply lifetime extension to the initializer of non-extended `super let`.
+            // This helps ensure that `{ super let x = &$EXPR; x }` is equivalent to `&$EXPR` in
+            // non-extending contexts: we want to avoid extending temporaries in `$EXPR` past what
+            // their temporary scopes would otherwise be (#145784).
+            // Currently, this shouldn't do anything. The discrepancy in #145784 was due to
+            // `{ super let x = &{ &temp() }; x }` extending `temp()` to outlive its immediately
+            // enclosing temporary scope (the block tail expression in Rust 2024), whereas in a
+            // non-extending context, `&{ &temp() }` would drop `temp()` at the end of the block.
+            // This particular quirk no longer exists: lifetime extension rules are applied to block
+            // tail expressions, so `temp()` is extended past the block in the latter case as well.
+            extend_initializer = false;
         }
     }
 
-    if let Some(expr) = init {
+    if let Some(expr) = init
+        && extend_initializer
+    {
         record_rvalue_scope_if_borrow_expr(visitor, expr, visitor.cx.var_parent);
 
         if let Some(pat) = pat {
@@ -592,87 +625,89 @@ fn resolve_local<'tcx>(
             | PatKind::Err(_) => false,
         }
     }
+}
 
-    /// If `expr` matches the `E&` grammar, then records an extended rvalue scope as appropriate:
-    ///
-    /// ```text
-    ///     E& = & ET
-    ///        | StructName { ..., f: E&, ... }
-    ///        | [ ..., E&, ... ]
-    ///        | ( ..., E&, ... )
-    ///        | {...; E&}
-    ///        | { super let ... = E&; ... }
-    ///        | if _ { ...; E& } else { ...; E& }
-    ///        | match _ { ..., _ => E&, ... }
-    ///        | box E&
-    ///        | E& as ...
-    ///        | ( E& )
-    /// ```
-    fn record_rvalue_scope_if_borrow_expr<'tcx>(
-        visitor: &mut ScopeResolutionVisitor<'tcx>,
-        expr: &hir::Expr<'_>,
-        blk_id: Option<Scope>,
-    ) {
-        match expr.kind {
-            hir::ExprKind::AddrOf(_, _, subexpr) => {
-                record_rvalue_scope_if_borrow_expr(visitor, subexpr, blk_id);
-                visitor.scope_tree.record_rvalue_candidate(
-                    subexpr.hir_id,
-                    RvalueCandidate { target: subexpr.hir_id.local_id, lifetime: blk_id },
-                );
-            }
-            hir::ExprKind::Struct(_, fields, _) => {
-                for field in fields {
-                    record_rvalue_scope_if_borrow_expr(visitor, field.expr, blk_id);
-                }
+/// If `expr` matches the `E&` grammar, then records an extended rvalue scope as appropriate:
+///
+/// ```text
+///     E& = & ET
+///        | StructName { ..., f: E&, ... }
+///        | Constructor(..., E&, ...)
+///        | [ ..., E&, ... ]
+///        | ( ..., E&, ... )
+///        | {...; E&}
+///        | { super let ... = E&; ... }
+///        | if _ { ...; E& } else { ...; E& }
+///        | match _ { ..., _ => E&, ... }
+///        | E& as ...
+/// ```
+fn record_rvalue_scope_if_borrow_expr<'tcx>(
+    visitor: &mut ScopeResolutionVisitor<'tcx>,
+    expr: &hir::Expr<'_>,
+    blk_id: Option<Scope>,
+) {
+    match expr.kind {
+        hir::ExprKind::AddrOf(_, _, subexpr) => {
+            record_rvalue_scope_if_borrow_expr(visitor, subexpr, blk_id);
+            visitor.scope_tree.record_rvalue_candidate(
+                subexpr.hir_id,
+                RvalueCandidate { target: subexpr.hir_id.local_id, lifetime: blk_id },
+            );
+        }
+        hir::ExprKind::Struct(_, fields, _) => {
+            for field in fields {
+                record_rvalue_scope_if_borrow_expr(visitor, field.expr, blk_id);
             }
-            hir::ExprKind::Array(subexprs) | hir::ExprKind::Tup(subexprs) => {
-                for subexpr in subexprs {
-                    record_rvalue_scope_if_borrow_expr(visitor, subexpr, blk_id);
-                }
+        }
+        hir::ExprKind::Array(subexprs) | hir::ExprKind::Tup(subexprs) => {
+            for subexpr in subexprs {
+                record_rvalue_scope_if_borrow_expr(visitor, subexpr, blk_id);
             }
-            hir::ExprKind::Cast(subexpr, _) => {
-                record_rvalue_scope_if_borrow_expr(visitor, subexpr, blk_id)
+        }
+        hir::ExprKind::Cast(subexpr, _) => {
+            record_rvalue_scope_if_borrow_expr(visitor, subexpr, blk_id)
+        }
+        hir::ExprKind::Block(block, _) => {
+            // Mark the block as extending, so we know its extending borrows and `super let`s
+            // have extended scopes when checking the block itself.
+            visitor.extended_blocks.insert(block.hir_id.local_id);
+            if let Some(subexpr) = block.expr {
+                record_rvalue_scope_if_borrow_expr(visitor, subexpr, blk_id);
             }
-            hir::ExprKind::Block(block, _) => {
-                if let Some(subexpr) = block.expr {
-                    record_rvalue_scope_if_borrow_expr(visitor, subexpr, blk_id);
-                }
-                for stmt in block.stmts {
-                    if let hir::StmtKind::Let(local) = stmt.kind
-                        && let Some(_) = local.super_
-                    {
-                        visitor.extended_super_lets.insert(local.pat.hir_id.local_id, blk_id);
-                    }
+            for stmt in block.stmts {
+                if let hir::StmtKind::Let(local) = stmt.kind
+                    && let Some(_) = local.super_
+                {
+                    visitor.extended_super_lets.insert(local.pat.hir_id.local_id, blk_id);
                 }
             }
-            hir::ExprKind::If(_, then_block, else_block) => {
-                record_rvalue_scope_if_borrow_expr(visitor, then_block, blk_id);
-                if let Some(else_block) = else_block {
-                    record_rvalue_scope_if_borrow_expr(visitor, else_block, blk_id);
-                }
+        }
+        hir::ExprKind::If(_, then_block, else_block) => {
+            record_rvalue_scope_if_borrow_expr(visitor, then_block, blk_id);
+            if let Some(else_block) = else_block {
+                record_rvalue_scope_if_borrow_expr(visitor, else_block, blk_id);
             }
-            hir::ExprKind::Match(_, arms, _) => {
-                for arm in arms {
-                    record_rvalue_scope_if_borrow_expr(visitor, arm.body, blk_id);
-                }
+        }
+        hir::ExprKind::Match(_, arms, _) => {
+            for arm in arms {
+                record_rvalue_scope_if_borrow_expr(visitor, arm.body, blk_id);
             }
-            hir::ExprKind::Call(func, args) => {
-                // Recurse into tuple constructors, such as `Some(&temp())`.
-                //
-                // That way, there is no difference between `Some(..)` and `Some { 0: .. }`,
-                // even though the former is syntactically a function call.
-                if let hir::ExprKind::Path(path) = &func.kind
-                    && let hir::QPath::Resolved(None, path) = path
-                    && let Res::SelfCtor(_) | Res::Def(DefKind::Ctor(_, CtorKind::Fn), _) = path.res
-                {
-                    for arg in args {
-                        record_rvalue_scope_if_borrow_expr(visitor, arg, blk_id);
-                    }
+        }
+        hir::ExprKind::Call(func, args) => {
+            // Recurse into tuple constructors, such as `Some(&temp())`.
+            //
+            // That way, there is no difference between `Some(..)` and `Some { 0: .. }`,
+            // even though the former is syntactically a function call.
+            if let hir::ExprKind::Path(path) = &func.kind
+                && let hir::QPath::Resolved(None, path) = path
+                && let Res::SelfCtor(_) | Res::Def(DefKind::Ctor(_, CtorKind::Fn), _) = path.res
+            {
+                for arg in args {
+                    record_rvalue_scope_if_borrow_expr(visitor, arg, blk_id);
                 }
             }
-            _ => {}
         }
+        _ => {}
     }
 }
 
@@ -813,6 +848,7 @@ pub(crate) fn region_scope_tree(tcx: TyCtxt<'_>, def_id: DefId) -> &ScopeTree {
             tcx,
             scope_tree: ScopeTree::default(),
             cx: Context { parent: None, var_parent: None },
+            extended_blocks: Default::default(),
             extended_super_lets: Default::default(),
         };
 

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/mir-opt/pre-codegen/slice_index.slice_index_range.PreCodegen.after.panic-unwind.mir

@@ -66,8 +66,8 @@ fn slice_index_range(_1: &[u32], _2: std::ops::Range<usize>) -> &[u32] {
         StorageDead(_10);
         StorageDead(_9);
         _0 = &(*_12);
-        StorageDead(_12);
         StorageDead(_8);
+        StorageDead(_12);
         return;
     }
 

tests/ui/borrowck/format-args-temporary-scopes.e2024.stderr

@@ -0,0 +1,27 @@
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/format-args-temporary-scopes.rs:18:48
+   |
+LL |     println!("{:?}", { std::convert::identity(&temp()) });
+   |                      --------------------------^^^^^^---
+   |                      |                         |     |
+   |                      |                         |     temporary value is freed at the end of this statement
+   |                      |                         creates a temporary value which is freed while still in use
+   |                      borrow later used here
+   |
+   = note: consider using a `let` binding to create a longer lived value
+
+error[E0716]: temporary value dropped while borrowed
+  --> $DIR/format-args-temporary-scopes.rs:25:52
+   |
+LL |     println!("{:?}{:?}", { std::convert::identity(&temp()) }, ());
+   |                          --------------------------^^^^^^---
+   |                          |                         |     |
+   |                          |                         |     temporary value is freed at the end of this statement
+   |                          |                         creates a temporary value which is freed while still in use
+   |                          borrow later used here
+   |
+   = note: consider using a `let` binding to create a longer lived value
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0716`.