extend borrow operators' operands' temporary scopes

Author: dianne

compiler/rustc_hir_analysis/src/check/region.rs

@@ -29,7 +29,8 @@ struct Context {
     parent: Option<Scope>,
 
     /// Scope of lifetime-extended temporaries. If `None`, extendable expressions have their usual
-    /// temporary scopes.
+    /// temporary scopes. Distinguishing the case of non-extended temporaries helps minimize the
+    /// number of extended lifetimes we record in the [`ScopeTree`]'s `rvalue_scopes` map.
     extended_parent: Option<ExtendedScope>,
 }
 
@@ -47,9 +48,32 @@ struct NodeInfo {
     extending: bool,
 }
 
-/// Scope of lifetime-extended temporaries. If the field is `None`, no drop is scheduled for them.
+/// Scope of lifetime-extended temporaries.
 #[derive(Debug, Copy, Clone)]
-struct ExtendedScope(Option<Scope>);
+enum ExtendedScope {
+    /// Extendable temporaries' scopes will be extended to match the scope of a `let` statement's
+    /// bindings, a `const`/`static` item, or a `const` block result. In the case of temporaries
+    /// extended by `const`s and `static`s, the field is `None`, meaning no drop is scheduled.
+    ThroughDeclaration(Option<Scope>),
+    /// Extendable temporaries will be dropped in the temporary scope enclosing the given scope.
+    /// This is a separate variant to minimize calls to [`ScopeTree::default_temporary_scope`].
+    ThroughExpression(Scope),
+}
+
+impl ExtendedScope {
+    fn to_scope(self, scope_tree: &ScopeTree) -> TempLifetime {
+        match self {
+            ExtendedScope::ThroughDeclaration(temp_lifetime) => {
+                TempLifetime { temp_lifetime, backwards_incompatible: None }
+            }
+            ExtendedScope::ThroughExpression(non_extending_parent) => {
+                let (temp_scope, backwards_incompatible) =
+                    scope_tree.default_temporary_scope(non_extending_parent);
+                TempLifetime { temp_lifetime: Some(temp_scope), backwards_incompatible }
+            }
+        }
+    }
+}
 
 struct ScopeResolutionVisitor<'tcx> {
     tcx: TyCtxt<'tcx>,
@@ -178,6 +202,14 @@ fn resolve_block<'tcx>(
                     .scope_tree
                     .backwards_incompatible_scope
                     .insert(local_id, Scope { local_id, data: ScopeData::Node });
+
+                // To avoid false positives in `tail_expr_drop_order`, make sure extendable
+                // temporaries are extended past the block tail even if that doesn't change their
+                // scopes in the current edition.
+                if visitor.cx.extended_parent.is_none() {
+                    visitor.cx.extended_parent =
+                        Some(ExtendedScope::ThroughExpression(visitor.cx.parent.unwrap()));
+                }
             }
             resolve_expr(visitor, tail_expr, NodeInfo { drop_temps, extending: true });
         }
@@ -296,8 +328,9 @@ fn resolve_expr<'tcx>(
     //        | E& as ...
     match expr.kind {
         hir::ExprKind::AddrOf(_, _, subexpr) => {
-            // TODO: generalize
-            if let Some(ExtendedScope(lifetime)) = visitor.cx.extended_parent {
+            // Record an extended lifetime for the operand if needed.
+            if let Some(extended_scope) = visitor.cx.extended_parent {
+                let lifetime = extended_scope.to_scope(&visitor.scope_tree);
                 record_subexpr_extended_temp_scopes(&mut visitor.scope_tree, subexpr, lifetime);
             }
             resolve_expr(visitor, subexpr, NodeInfo { drop_temps: false, extending: true });
@@ -563,10 +596,9 @@ fn resolve_local<'tcx>(
         // FIXME(super_let): This ignores backward-incompatible drop hints. Implementing BIDs for
         // `super let` bindings could improve `tail_expr_drop_order` with regard to `pin!`, etc.
 
-        // TODO: generalize
         visitor.cx.var_parent = match visitor.cx.extended_parent {
             // If the extended parent scope was set, use it.
-            Some(ExtendedScope(lifetime)) => lifetime,
+            Some(extended_parent) => extended_parent.to_scope(&visitor.scope_tree).temp_lifetime,
             // Otherwise, like a temporaries, bindings are dropped in the enclosing temporary scope.
             None => visitor
                 .cx
@@ -582,7 +614,7 @@ fn resolve_local<'tcx>(
             record_subexpr_extended_temp_scopes(
                 &mut visitor.scope_tree,
                 expr,
-                visitor.cx.var_parent,
+                TempLifetime { temp_lifetime: visitor.cx.var_parent, backwards_incompatible: None },
             );
         }
 
@@ -592,7 +624,8 @@ fn resolve_local<'tcx>(
             // When visiting the initializer, extend borrows and `super let`s accessible through
             // extending subexpressions to live in the current variable scope (or in the case of
             // statics and consts, for the whole program).
-            visitor.cx.extended_parent = Some(ExtendedScope(visitor.cx.var_parent));
+            visitor.cx.extended_parent =
+                Some(ExtendedScope::ThroughDeclaration(visitor.cx.var_parent));
         }
 
         // Make sure we visit the initializer first.
@@ -694,7 +727,7 @@ fn resolve_local<'tcx>(
 fn record_subexpr_extended_temp_scopes(
     scope_tree: &mut ScopeTree,
     mut expr: &hir::Expr<'_>,
-    lifetime: Option<Scope>,
+    lifetime: TempLifetime,
 ) {
     debug!(?expr, ?lifetime);
 
@@ -741,6 +774,19 @@ impl<'tcx> ScopeResolutionVisitor<'tcx> {
         // account for the destruction scope representing the scope of
         // the destructors that run immediately after it completes.
         if node_info.drop_temps {
+            // If this scope corresponds to an extending subexpression, we can extend
+            // lifetime-extendible temporaries' scopes through it. If we're not already
+            // lifetime-extending to some larger scope, we set the `extended_parent` to use the
+            // temporary scope enclosing this node as the scope of lifetime-extended temporaries.
+            // We only do this for extending subexpressions that also drop their temporaries, rather
+            // than for all extending subxpressions, so that we only "lifetime-extend" when we're
+            // actually changing expressions' temporary scopes; this avoids unnecessary insertions
+            // to the `ScopeTree`'s table of expressions with extended temporary scopes.
+            if node_info.extending && self.cx.extended_parent.is_none() {
+                self.cx.extended_parent = Some(ExtendedScope::ThroughExpression(
+                    self.cx.parent.expect("extending subexpressions should have parent scopes"),
+                ));
+            }
             self.enter_scope(Scope { local_id: id, data: ScopeData::Destruction });
         }
         self.enter_scope(Scope { local_id: id, data: ScopeData::Node });

compiler/rustc_middle/src/middle/region.rs

@@ -226,7 +226,7 @@ pub struct ScopeTree {
     /// `rustc_hir_analysis::check::region` and in the [Reference].
     ///
     /// [Reference]: https://doc.rust-lang.org/nightly/reference/destructors.html#temporary-lifetime-extension
-    extended_temp_scopes: ItemLocalMap<Option<Scope>>,
+    extended_temp_scopes: ItemLocalMap<TempLifetime>,
 
     /// Backwards incompatible scoping that will be introduced in future editions.
     /// This information is used later for linting to identify locals and
@@ -235,7 +235,7 @@ pub struct ScopeTree {
 }
 
 /// Temporary lifetime information for expressions, used when lowering to MIR.
-#[derive(Clone, Copy, Debug, HashStable)]
+#[derive(Clone, Copy, PartialEq, Eq, Debug, HashStable)]
 pub struct TempLifetime {
     /// The scope in which a temporary should be dropped. If `None`, no drop is scheduled; this is
     /// the case for lifetime-extended temporaries extended by a const/static item or const block.
@@ -262,12 +262,13 @@ impl ScopeTree {
     }
 
     /// Make an association between a sub-expression and an extended lifetime
-    pub fn record_extended_temp_scope(&mut self, var: hir::ItemLocalId, lifetime: Option<Scope>) {
+    pub fn record_extended_temp_scope(&mut self, var: hir::ItemLocalId, lifetime: TempLifetime) {
         debug!(?var, ?lifetime);
-        if let Some(lifetime) = lifetime {
+        if let Some(lifetime) = lifetime.temp_lifetime {
             assert!(var != lifetime.local_id);
         }
-        self.extended_temp_scopes.insert(var, lifetime);
+        let old_lifetime = self.extended_temp_scopes.insert(var, lifetime);
+        assert!(old_lifetime.is_none_or(|old| old == lifetime));
     }
 
     /// Returns the narrowest scope that encloses `id`, if any.
@@ -347,7 +348,7 @@ impl ScopeTree {
         // Check for a designated extended temporary scope.
         if let Some(&s) = self.extended_temp_scopes.get(&expr_id) {
             debug!("temporary_scope({expr_id:?}) = {s:?} [custom]");
-            return TempLifetime { temp_lifetime: s, backwards_incompatible: None };
+            return s;
         }
 
         // Otherwise, locate the innermost terminating scope.

tests/mir-opt/pre-codegen/slice_index.slice_index_range.PreCodegen.after.panic-abort.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/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.e2021.stderr

@@ -1,30 +1,5 @@
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/format-args-temporary-scopes.rs:40:90
-   |
-LL |     println!("{:?}{:?}", (), match true { true => &"" as &dyn std::fmt::Debug, false => &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:33:41
-   |
-LL |     println!("{:?}{:?}", (), if true { &format!("") } else { "" });
-   |                              -----------^^^^^^^^^^^--------------
-   |                              |          |           |
-   |                              |          |           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
-   = note: this error originates in the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info)
-
-error[E0716]: temporary value dropped while borrowed
-  --> $DIR/format-args-temporary-scopes.rs:36:64
+  --> $DIR/format-args-temporary-scopes.rs:33:64
    |
 LL |     println!("{:?}{:?}", (), if true { std::convert::identity(&format!("")) } else { "" });
    |                              ----------------------------------^^^^^^^^^^^---------------
@@ -36,6 +11,6 @@ LL |     println!("{:?}{:?}", (), if true { std::convert::identity(&format!(""))
    = note: consider using a `let` binding to create a longer lived value
    = note: this error originates in the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error: aborting due to 3 previous errors
+error: aborting due to 1 previous error
 
 For more information about this error, try `rustc --explain E0716`.

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

@@ -1,15 +1,3 @@
-error[E0716]: temporary value dropped while borrowed
-  --> $DIR/format-args-temporary-scopes.rs:12:25
-   |
-LL |     println!("{:?}", { &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:17:48
    |
@@ -23,19 +11,7 @@ LL |     println!("{:?}", { std::convert::identity(&temp()) });
    = 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:23:29
-   |
-LL |     println!("{:?}{:?}", { &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:26:52
+  --> $DIR/format-args-temporary-scopes.rs:24:52
    |
 LL |     println!("{:?}{:?}", { std::convert::identity(&temp()) }, ());
    |                          --------------------------^^^^^^---
@@ -47,32 +23,7 @@ LL |     println!("{:?}{:?}", { std::convert::identity(&temp()) }, ());
    = 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:40:90
-   |
-LL |     println!("{:?}{:?}", (), match true { true => &"" as &dyn std::fmt::Debug, false => &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:33:41
-   |
-LL |     println!("{:?}{:?}", (), if true { &format!("") } else { "" });
-   |                                        -^^^^^^^^^^-
-   |                                        ||         |
-   |                                        ||         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
-   = note: this error originates in the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info)
-
-error[E0716]: temporary value dropped while borrowed
-  --> $DIR/format-args-temporary-scopes.rs:36:64
+  --> $DIR/format-args-temporary-scopes.rs:33:64
    |
 LL |     println!("{:?}{:?}", (), if true { std::convert::identity(&format!("")) } else { "" });
    |                                        ------------------------^^^^^^^^^^^-
@@ -84,6 +35,6 @@ LL |     println!("{:?}{:?}", (), if true { std::convert::identity(&format!(""))
    = note: consider using a `let` binding to create a longer lived value
    = note: this error originates in the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info)
 
-error: aborting due to 7 previous errors
+error: aborting due to 3 previous errors
 
 For more information about this error, try `rustc --explain E0716`.

tests/ui/borrowck/format-args-temporary-scopes.rs

@@ -7,21 +7,19 @@
 fn temp() {}
 
 fn main() {
-    // In Rust 2024, block tail expressions are temporary scopes, so the result of `temp()` is
-    // dropped after evaluating `&temp()`.
+    // In Rust 2024, block tail expressions are temporary scopes, but temporary lifetime extension
+    // rules apply: `&temp()` here is an extending borrow expression, so `temp()`'s lifetime is
+    // extended past the block.
     println!("{:?}", { &temp() });
-    //[e2024]~^ ERROR: temporary value dropped while borrowed [E0716]
 
     // Arguments to function calls aren't extending expressions, so `temp()` is dropped at the end
     // of the block in Rust 2024.
     println!("{:?}", { std::convert::identity(&temp()) });
     //[e2024]~^ ERROR: temporary value dropped while borrowed [E0716]
 
-    // In Rust 1.89, `format_args!` extended the lifetime of all extending expressions in its
-    // arguments when provided with two or more arguments. This caused the result of `temp()` to
-    // outlive the result of the block, making this compile.
+    // In Rust 1.89, `format_args!` had different lifetime extension behavior dependent on how many
+    // formatting arguments it had (#145880), so let's test that too.
     println!("{:?}{:?}", { &temp() }, ());
-    //[e2024]~^ ERROR: temporary value dropped while borrowed [E0716]
 
     println!("{:?}{:?}", { std::convert::identity(&temp()) }, ());
     //[e2024]~^ ERROR: temporary value dropped while borrowed [E0716]
@@ -31,12 +29,10 @@ fn main() {
     // blocks of `if` expressions are temporary scopes in all editions, this affects Rust 2021 and
     // earlier as well.
     println!("{:?}{:?}", (), if true { &format!("") } else { "" });
-    //~^ ERROR: temporary value dropped while borrowed [E0716]
 
     println!("{:?}{:?}", (), if true { std::convert::identity(&format!("")) } else { "" });
     //~^ ERROR: temporary value dropped while borrowed [E0716]
 
     // This has likewise occurred with `match`, affecting all editions.
     println!("{:?}{:?}", (), match true { true => &"" as &dyn std::fmt::Debug, false => &temp() });
-    //~^ ERROR: temporary value dropped while borrowed [E0716]
 }

tests/ui/borrowck/super-let-in-if-block.rs

@@ -1,5 +1,6 @@
 //! Test that `super let` bindings in `if` expressions' blocks have the same scope as the result
 //! of the block.
+//@ check-pass
 #![feature(super_let)]
 
 fn main() {
@@ -16,14 +17,11 @@ fn main() {
 
     // For `super let` in non-extending `if`, the binding `temp` should live in the temporary scope
     // the `if` expression is in.
-    // TODO: make this not an error
     std::convert::identity(if true {
         super let temp = ();
         &temp
-        //~^ ERROR `temp` does not live long enough
     } else {
         super let temp = ();
         &temp
-        //~^ ERROR `temp` does not live long enough
     });
 }