lifetime-extend extending borrows in block tail expressions

Author: dianne

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);
         }
     }
@@ -470,7 +492,7 @@ fn resolve_local<'tcx>(
     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();
@@ -483,7 +505,8 @@ 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();
             //
@@ -494,10 +517,16 @@ fn resolve_local<'tcx>(
             if let Some(inner_scope) = visitor.cx.var_parent {
                 (visitor.cx.var_parent, _) = visitor.scope_tree.default_temporary_scope(inner_scope)
             }
-            // Don't lifetime-extend child `super let`s or block tail expressions' temporaries in
-            // the initializer when this `super let` is not itself extended by a parent `let`
-            // (#145784). Block tail expressions are temporary drop scopes in Editions 2024 and
-            // later, their temps shouldn't outlive the block in e.g. `f(pin!({ &temp() }))`.
+            // 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;
         }
     }
@@ -639,6 +668,9 @@ fn record_rvalue_scope_if_borrow_expr<'tcx>(
             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);
             }
@@ -816,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(),
         };
 

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

@@ -1,23 +1,23 @@
 error[E0716]: temporary value dropped while borrowed
-  --> $DIR/format-args-temporary-scopes.rs:13:25
+  --> $DIR/format-args-temporary-scopes.rs:18:48
    |
-LL |     println!("{:?}", { &temp() });
-   |                      ---^^^^^---
-   |                      |  |    |
-   |                      |  |    temporary value is freed at the end of this statement
-   |                      |  creates a temporary value which is freed while still in use
+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:19:29
+  --> $DIR/format-args-temporary-scopes.rs:25:52
    |
-LL |     println!("{:?}{:?}", { &temp() }, ());
-   |                          ---^^^^^---
-   |                          |  |    |
-   |                          |  |    temporary value is freed at the end of this statement
-   |                          |  creates a temporary value which is freed while still in use
+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

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

@@ -8,14 +8,20 @@
 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() });
+
+    // 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() }, ());
+
+    println!("{:?}{:?}", { std::convert::identity(&temp()) }, ());
     //[e2024]~^ ERROR: temporary value dropped while borrowed [E0716]
 }

tests/ui/drop/destructuring-assignments.rs

@@ -14,11 +14,11 @@
 
 fn main() {
     assert_drop_order(1..=3, |e| {
-        &({ &raw const *&e.log(1) }, drop(e.log(2)));
+        &({ &raw const *&e.log(2) }, drop(e.log(1)));
         drop(e.log(3));
     });
     assert_drop_order(1..=3, |e| {
-        { let _x; _x = &({ &raw const *&e.log(1) }, drop(e.log(2))); }
+        { let _x; _x = &({ &raw const *&e.log(2) }, drop(e.log(1))); }
         drop(e.log(3));
     });
     assert_drop_order(1..=3, |e| {

tests/ui/drop/lint-tail-expr-drop-order-borrowck.rs

@@ -28,7 +28,7 @@ fn should_lint_with_unsafe_block() {
 fn should_lint_with_big_block() {
     fn f<T>(_: T) {}
     f({
-        &mut || 0
+        std::convert::identity(&mut || 0)
         //~^ ERROR: relative drop order changing
         //~| WARN: this changes meaning in Rust 2024
         //~| NOTE: this temporary value will be dropped at the end of the block
@@ -40,12 +40,17 @@ fn should_lint_with_big_block() {
 fn another_temp_that_is_copy_in_arg() {
     fn f() {}
     fn g(_: &()) {}
-    g({ &f() });
+    g({ std::convert::identity(&f()) });
     //~^ ERROR: relative drop order changing
     //~| WARN: this changes meaning in Rust 2024
     //~| NOTE: this temporary value will be dropped at the end of the block
     //~| NOTE: borrow later used by call
     //~| NOTE: for more information, see
 }
 
+fn do_not_lint_extending_borrow() {
+    fn f<T>(_: T) {}
+    f({ &mut || 0 });
+}
+
 fn main() {}

tests/ui/drop/lint-tail-expr-drop-order-borrowck.stderr

@@ -26,22 +26,22 @@ LL |     f(unsafe { String::new().as_str() }.len());
    = note: for more information, see <https://doc.rust-lang.org/edition-guide/rust-2024/temporary-tail-expr-scope.html>
 
 error: relative drop order changing in Rust 2024
-  --> $DIR/lint-tail-expr-drop-order-borrowck.rs:31:9
+  --> $DIR/lint-tail-expr-drop-order-borrowck.rs:31:32
    |
-LL |         &mut || 0
-   |         ^^^^^^^^^
-   |         |
-   |         this temporary value will be dropped at the end of the block
+LL |         std::convert::identity(&mut || 0)
+   |         -----------------------^^^^^^^^^-
+   |         |                      |
+   |         |                      this temporary value will be dropped at the end of the block
    |         borrow later used here
    |
    = warning: this changes meaning in Rust 2024
    = note: for more information, see <https://doc.rust-lang.org/edition-guide/rust-2024/temporary-tail-expr-scope.html>
 
 error: relative drop order changing in Rust 2024
-  --> $DIR/lint-tail-expr-drop-order-borrowck.rs:43:9
+  --> $DIR/lint-tail-expr-drop-order-borrowck.rs:43:32
    |
-LL |     g({ &f() });
-   |     -   ^^^^ this temporary value will be dropped at the end of the block
+LL |     g({ std::convert::identity(&f()) });
+   |     -                          ^^^^ this temporary value will be dropped at the end of the block
    |     |
    |     borrow later used by call
    |

tests/ui/drop/super-let-tail-expr-drop-order.rs

@@ -1,18 +1,19 @@
-//! Test for #145784: the argument to `pin!` should be treated as an extending expression if and
-//! only if the whole `pin!` invocation is an extending expression. Likewise, since `pin!` is
-//! implemented in terms of `super let`, test the same for `super let` initializers. Since the
-//! argument to `pin!` and the initializer of `super let` are not temporary drop scopes, this only
-//! affects lifetimes in two cases:
+//! Test for #145784. This tests three things:
 //!
-//! - Block tail expressions in Rust 2024, which are both extending expressions and temporary drop
-//! scopes; treating them as extending expressions within a non-extending `pin!` resulted in borrow
-//! expression operands living past the end of the block.
+//! - In Rust 2024, temporary lifetime extension applies to block tail expressions, such that
+//!   extending borrows and `super let`s in block tails are extended to outlive the result of the
+//!   block.
 //!
-//! - Nested `super let` statements, which can have their binding and temporary lifetimes extended
-//! when the block they're in is an extending expression.
+//! - Since `super let`'s initializer has the same temporary scope as the variable scope of its
+//!   bindings, this means that lifetime extension can effectively see through `super let`.
 //!
-//! For more information on extending expressions, see
-//! https://doc.rust-lang.org/reference/destructors.html#extending-based-on-expressions
+//! - In particular, the argument to `pin!` is an extending expression, and the argument of an
+//!   extending `pin!` has an extended temporary scope. The lifetime of the argument, as well those
+//!   of extending borrows and `super lets` within it, should match the result of the `pin!`,
+//!   regardless of whether it itself is extended by a parent expression.
+//!
+//! For more information on temporary lifetime extension, see
+//! https://doc.rust-lang.org/nightly/reference/destructors.html#temporary-lifetime-extension
 //!
 //! For tests that `super let` initializers aren't temporary drop scopes, and tests for
 //! lifetime-extended `super let`, see tests/ui/borrowck/super-let-lifetime-and-drop.rs
@@ -30,34 +31,25 @@ use std::pin::pin;
 fn f<T>(_: LogDrop<'_>, x: T) -> T { x }
 
 fn main() {
-    // Test block arguments to `pin!` in non-extending expressions.
+    // Test block arguments to non-extending `pin!`.
     // In Rust 2021, block tail expressions aren't temporary drop scopes, so their temporaries
     // should outlive the `pin!` invocation.
-    // In Rust 2024, block tail expressions are temporary drop scopes, so their temporaries should
-    // be dropped after evaluating the tail expression within the `pin!` invocation.
-    // By nesting two `pin!` calls, this ensures non-extended `pin!` doesn't extend an inner `pin!`.
+    // In Rust 2024, extending borrows within block tail expressions have extended lifetimes to
+    // outlive result of the block, so the end result is the same in this case.
+    // By nesting two `pin!` calls, this ensures extending borrows in the inner `pin!` outlive the
+    // outer `pin!`.
     assert_drop_order(1..=3, |o| {
-        #[cfg(e2021)]
         (
             pin!((
                 pin!({ &o.log(3) as *const LogDrop<'_> }),
                 drop(o.log(1)),
             )),
             drop(o.log(2)),
         );
-        #[cfg(e2024)]
-        (
-            pin!((
-                pin!({ &o.log(1) as *const LogDrop<'_> }),
-                drop(o.log(2)),
-            )),
-            drop(o.log(3)),
-        );
     });
 
     // The same holds for `super let` initializers in non-extending expressions.
     assert_drop_order(1..=4, |o| {
-        #[cfg(e2021)]
         (
             {
                 super let _ = {
@@ -68,17 +60,6 @@ fn main() {
             },
             drop(o.log(3)),
         );
-        #[cfg(e2024)]
-        (
-            {
-                super let _ = {
-                    super let _ = { &o.log(1) as *const LogDrop<'_> };
-                    drop(o.log(2))
-                };
-                drop(o.log(3))
-            },
-            drop(o.log(4)),
-        );
     });
 
     // Within an extending expression, the argument to `pin!` is also an extending expression,
@@ -97,36 +78,18 @@ fn main() {
     // We have extending borrow expressions within an extending block
     // expression (within an extending borrow expression) within a
     // non-extending expresion within the initializer expression.
-    #[cfg(e2021)]
-    {
-        // These two should be the same.
-        assert_drop_order(1..=3, |e| {
-            let _v = f(e.log(1), &{ &raw const *&e.log(2) });
-            drop(e.log(3));
-        });
-        assert_drop_order(1..=3, |e| {
-            let _v = f(e.log(1), {
-                super let v = &{ &raw const *&e.log(2) };
-                v
-            });
-            drop(e.log(3));
-        });
-    }
-    #[cfg(e2024)]
-    {
-        // These two should be the same.
-        assert_drop_order(1..=3, |e| {
-            let _v = f(e.log(2), &{ &raw const *&e.log(1) });
-            drop(e.log(3));
-        });
-        assert_drop_order(1..=3, |e| {
-            let _v = f(e.log(2), {
-                super let v = &{ &raw const *&e.log(1) };
-                v
-            });
-            drop(e.log(3));
+    // These two should be the same.
+    assert_drop_order(1..=3, |e| {
+        let _v = f(e.log(1), &{ &raw const *&e.log(2) });
+        drop(e.log(3));
+    });
+    assert_drop_order(1..=3, |e| {
+        let _v = f(e.log(1), {
+            super let v = &{ &raw const *&e.log(2) };
+            v
         });
-    }
+        drop(e.log(3));
+    });
 
     // We have extending borrow expressions within a non-extending
     // expression within the initializer expression.