Skip to content

Commit e649bbe

Browse files
diannedianne
committed
lifetime extension for blocks
1 parent 707fd66 commit e649bbe

10 files changed

+147
-147
lines changed

‎compiler/rustc_hir_analysis/src/check/region.rs‎

Lines changed: 63 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
99
use std::mem;
1010

11-
use rustc_data_structures::fx::FxHashMap;
11+
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
1212
use rustc_hir as hir;
1313
use rustc_hir::def::{CtorKind, DefKind, Res};
1414
use rustc_hir::def_id::DefId;
@@ -38,6 +38,14 @@ struct ScopeResolutionVisitor<'tcx> {
3838

3939
cx: Context,
4040

41+
/// Tracks [extending] blocks and `if` expressions. This is used in performing lifetime
42+
/// extension on block tail expressions: if we've already extended the temporary scopes of
43+
/// extending borrows within a block's tail when checking a parent `let` statement or block, we
44+
/// don't want to re-extend them to be shorter when checking the block itself.
45+
///
46+
/// [extending]: https://doc.rust-lang.org/nightly/reference/destructors.html#extending-based-on-expressions
47+
extended_blocks: FxHashSet<hir::ItemLocalId>,
48+
4149
extended_super_lets: FxHashMap<hir::ItemLocalId, Option<Scope>>,
4250
}
4351

@@ -160,6 +168,17 @@ fn resolve_block<'tcx>(
160168
.backwards_incompatible_scope
161169
.insert(local_id, Scope { local_id, data: ScopeData::Node });
162170
}
171+
// If we haven't already checked for temporary lifetime extension due to a parent `let`
172+
// statement initializer or block, do so. This, e.g., allows `temp()` in `{ &temp() }`
173+
// to outlive the block even when the block itself is not in a `let` statement
174+
// initializer. The same rules for `let` are used here, so non-extending borrows are
175+
// unaffected: `{ f(&temp()) }` drops `temp()` at the end of the block in Rust 2024.
176+
if !visitor.extended_blocks.contains(&blk.hir_id.local_id) {
177+
let blk_result_scope = prev_cx.parent.and_then(|blk_parent| {
178+
visitor.scope_tree.default_temporary_scope(blk_parent).0
179+
});
180+
record_rvalue_scope_if_borrow_expr(visitor, tail_expr, blk_result_scope);
181+
}
163182
resolve_expr(visitor, tail_expr, terminating);
164183
}
165184
}
@@ -354,6 +373,22 @@ fn resolve_expr<'tcx>(
354373

355374
hir::ExprKind::If(cond, then, Some(otherwise)) => {
356375
let expr_cx = visitor.cx;
376+
// If we haven't already checked for temporary lifetime extension due to a parent `let`
377+
// statement initializer or block, do so. This, e.g., allows `format!("temp")` in
378+
// `if cond { &format!("temp") } else { "" }` to outlive the block even when the `if`
379+
// expression itself is not in a `let` statement initializer. The same rules for `let`
380+
// are used here, so non-extending borrows are unaffected:
381+
// `if cond { f(&format!("temp")) } else { "" }`
382+
// drops `format!("temp")` at the end of the block in all editions.
383+
// This also allows `super let` in the then and else blocks to have the scope of the
384+
// result of the block, as expected.
385+
if !visitor.extended_blocks.contains(&expr.hir_id.local_id) {
386+
let blk_result_scope = expr_cx
387+
.parent
388+
.and_then(|if_parent| visitor.scope_tree.default_temporary_scope(if_parent).0);
389+
record_rvalue_scope_if_borrow_expr(visitor, then, blk_result_scope);
390+
record_rvalue_scope_if_borrow_expr(visitor, otherwise, blk_result_scope);
391+
}
357392
let data = if expr.span.at_least_rust_2024() {
358393
ScopeData::IfThenRescope
359394
} else {
@@ -369,6 +404,13 @@ fn resolve_expr<'tcx>(
369404

370405
hir::ExprKind::If(cond, then, None) => {
371406
let expr_cx = visitor.cx;
407+
// As above, perform lifetime extension on the consequent block.
408+
if !visitor.extended_blocks.contains(&expr.hir_id.local_id) {
409+
let blk_result_scope = expr_cx
410+
.parent
411+
.and_then(|if_parent| visitor.scope_tree.default_temporary_scope(if_parent).0);
412+
record_rvalue_scope_if_borrow_expr(visitor, then, blk_result_scope);
413+
}
372414
let data = if expr.span.at_least_rust_2024() {
373415
ScopeData::IfThenRescope
374416
} else {
@@ -473,7 +515,7 @@ fn resolve_local<'tcx>(
473515
if let Some(scope) =
474516
visitor.extended_super_lets.remove(&pat.unwrap().hir_id.local_id) =>
475517
{
476-
// This expression was lifetime-extended by a parent let binding. E.g.
518+
// This expression was lifetime-extended by a parent let binding or block. E.g.
477519
//
478520
// let a = {
479521
// super let b = temp();
@@ -489,7 +531,8 @@ fn resolve_local<'tcx>(
489531
true
490532
}
491533
LetKind::Super => {
492-
// This `super let` is not subject to lifetime extension from a parent let binding. E.g.
534+
// This `super let` is not subject to lifetime extension from a parent let binding or
535+
// block. E.g.
493536
//
494537
// identity({ super let x = temp(); &x }).method();
495538
//
@@ -500,10 +543,16 @@ fn resolve_local<'tcx>(
500543
if let Some(inner_scope) = visitor.cx.var_parent {
501544
(visitor.cx.var_parent, _) = visitor.scope_tree.default_temporary_scope(inner_scope)
502545
}
503-
// Don't lifetime-extend child `super let`s or block tail expressions' temporaries in
504-
// the initializer when this `super let` is not itself extended by a parent `let`
505-
// (#145784). Block tail expressions are temporary drop scopes in Editions 2024 and
506-
// later, their temps shouldn't outlive the block in e.g. `f(pin!({ &temp() }))`.
546+
// Don't apply lifetime extension to the initializer of non-extended `super let`.
547+
// This helps ensure that `{ super let x = &$EXPR; x }` is equivalent to `&$EXPR` in
548+
// non-extending contexts: we want to avoid extending temporaries in `$EXPR` past what
549+
// their temporary scopes would otherwise be (#145784).
550+
// Currently, this shouldn't do anything. The discrepancy in #145784 was due to
551+
// `{ super let x = &{ &temp() }; x }` extending `temp()` to outlive its immediately
552+
// enclosing temporary scope (the block tail expression in Rust 2024), whereas in a
553+
// non-extending context, `&{ &temp() }` would drop `temp()` at the end of the block.
554+
// This particular quirk no longer exists: lifetime extension rules are applied to block
555+
// tail expressions, so `temp()` is extended past the block in the latter case as well.
507556
false
508557
}
509558
};
@@ -645,6 +694,9 @@ fn record_rvalue_scope_if_borrow_expr<'tcx>(
645694
record_rvalue_scope_if_borrow_expr(visitor, subexpr, blk_id)
646695
}
647696
hir::ExprKind::Block(block, _) => {
697+
// Mark the block as extending, so we know its extending borrows and `super let`s have
698+
// extended scopes when checking the block itself.
699+
visitor.extended_blocks.insert(block.hir_id.local_id);
648700
if let Some(subexpr) = block.expr {
649701
record_rvalue_scope_if_borrow_expr(visitor, subexpr, blk_id);
650702
}
@@ -657,6 +709,9 @@ fn record_rvalue_scope_if_borrow_expr<'tcx>(
657709
}
658710
}
659711
hir::ExprKind::If(_, then_block, else_block) => {
712+
// Mark the expression as extending, so we know its extending borrows and `super let`s
713+
// have extended scopes when checking the `if` expression's blocks.
714+
visitor.extended_blocks.insert(expr.hir_id.local_id);
660715
record_rvalue_scope_if_borrow_expr(visitor, then_block, blk_id);
661716
if let Some(else_block) = else_block {
662717
record_rvalue_scope_if_borrow_expr(visitor, else_block, blk_id);
@@ -822,6 +877,7 @@ pub(crate) fn region_scope_tree(tcx: TyCtxt<'_>, def_id: DefId) -> &ScopeTree {
822877
tcx,
823878
scope_tree: ScopeTree::default(),
824879
cx: Context { parent: None, var_parent: None },
880+
extended_blocks: Default::default(),
825881
extended_super_lets: Default::default(),
826882
};
827883

‎tests/ui/borrowck/format-args-temporary-scopes.e2021.stderr‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
error[E0716]: temporary value dropped while borrowed
2-
--> $DIR/format-args-temporary-scopes.rs:25:41
2+
--> $DIR/format-args-temporary-scopes.rs:33:64
33
|
4-
LL | println!("{:?}{:?}", (), if true { &format!("") } else { "" });
5-
| -----------^^^^^^^^^^^--------------
6-
| | | |
7-
| | | temporary value is freed at the end of this statement
8-
| | creates a temporary value which is freed while still in use
4+
LL | println!("{:?}{:?}", (), if true { std::convert::identity(&format!("")) } else { "" });
5+
| ----------------------------------^^^^^^^^^^^---------------
6+
| | | |
7+
| | | temporary value is freed at the end of this statement
8+
| | creates a temporary value which is freed while still in use
99
| borrow later used here
1010
|
1111
= note: consider using a `let` binding to create a longer lived value

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

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,35 @@
11
error[E0716]: temporary value dropped while borrowed
2-
--> $DIR/format-args-temporary-scopes.rs:12:25
2+
--> $DIR/format-args-temporary-scopes.rs:17:48
33
|
4-
LL | println!("{:?}", { &temp() });
5-
| ---^^^^^---
6-
| | | |
7-
| | | temporary value is freed at the end of this statement
8-
| | creates a temporary value which is freed while still in use
4+
LL | println!("{:?}", { std::convert::identity(&temp()) });
5+
| --------------------------^^^^^^---
6+
| | | |
7+
| | | temporary value is freed at the end of this statement
8+
| | creates a temporary value which is freed while still in use
99
| borrow later used here
1010
|
1111
= note: consider using a `let` binding to create a longer lived value
1212

1313
error[E0716]: temporary value dropped while borrowed
14-
--> $DIR/format-args-temporary-scopes.rs:18:29
14+
--> $DIR/format-args-temporary-scopes.rs:24:52
1515
|
16-
LL | println!("{:?}{:?}", { &temp() }, ());
17-
| ---^^^^^---
18-
| | | |
19-
| | | temporary value is freed at the end of this statement
20-
| | creates a temporary value which is freed while still in use
16+
LL | println!("{:?}{:?}", { std::convert::identity(&temp()) }, ());
17+
| --------------------------^^^^^^---
18+
| | | |
19+
| | | temporary value is freed at the end of this statement
20+
| | creates a temporary value which is freed while still in use
2121
| borrow later used here
2222
|
2323
= note: consider using a `let` binding to create a longer lived value
2424

2525
error[E0716]: temporary value dropped while borrowed
26-
--> $DIR/format-args-temporary-scopes.rs:25:41
26+
--> $DIR/format-args-temporary-scopes.rs:33:64
2727
|
28-
LL | println!("{:?}{:?}", (), if true { &format!("") } else { "" });
29-
| -^^^^^^^^^^-
30-
| || |
31-
| || temporary value is freed at the end of this statement
32-
| |creates a temporary value which is freed while still in use
28+
LL | println!("{:?}{:?}", (), if true { std::convert::identity(&format!("")) } else { "" });
29+
| ------------------------^^^^^^^^^^^-
30+
| | | |
31+
| | | temporary value is freed at the end of this statement
32+
| | creates a temporary value which is freed while still in use
3333
| borrow later used here
3434
|
3535
= note: consider using a `let` binding to create a longer lived value

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,29 @@
77
fn temp() {}
88

99
fn main() {
10-
// In Rust 2024, block tail expressions are temporary scopes, so the result of `temp()` is
11-
// dropped after evaluating `&temp()`.
10+
// In Rust 2024, block tail expressions are temporary scopes, but temporary lifetime extension
11+
// rules apply: `&temp()` here is an extending borrow expression, so `temp()`'s lifetime is
12+
// extended past the block.
1213
println!("{:?}", { &temp() });
14+
15+
// Arguments to function calls aren't extending expressions, so `temp()` is dropped at the end
16+
// of the block in Rust 2024.
17+
println!("{:?}", { std::convert::identity(&temp()) });
1318
//[e2024]~^ ERROR: temporary value dropped while borrowed [E0716]
1419

15-
// In Rust 1.89, `format_args!` extended the lifetime of all extending expressions in its
16-
// arguments when provided with two or more arguments. This caused the result of `temp()` to
17-
// outlive the result of the block, making this compile.
20+
// In Rust 1.89, `format_args!` had different lifetime extension behavior dependent on how many
21+
// formatting arguments it had (#145880), so let's test that too.
1822
println!("{:?}{:?}", { &temp() }, ());
23+
24+
println!("{:?}{:?}", { std::convert::identity(&temp()) }, ());
1925
//[e2024]~^ ERROR: temporary value dropped while borrowed [E0716]
2026

2127
// In real-world projects, this typically appeared in `if` expressions with a `&str` in one
2228
// branch and a reference to a `String` temporary in the other. Since the consequent and `else`
2329
// blocks of `if` expressions are temporary scopes in all editions, this affects Rust 2021 and
2430
// earlier as well.
2531
println!("{:?}{:?}", (), if true { &format!("") } else { "" });
32+
33+
println!("{:?}{:?}", (), if true { std::convert::identity(&format!("")) } else { "" });
2634
//~^ ERROR: temporary value dropped while borrowed [E0716]
2735
}

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//! Test that `super let` bindings in `if` expressions' blocks have the same scope as the result
22
//! of the block.
3+
//@ check-pass
34
#![feature(super_let)]
45

56
fn main() {
@@ -16,14 +17,11 @@ fn main() {
1617

1718
// For `super let` in non-extending `if`, the binding `temp` should live in the temporary scope
1819
// the `if` expression is in.
19-
// TODO: make this not an error
2020
std::convert::identity(if true {
2121
super let temp = ();
2222
&temp
23-
//~^ ERROR `temp` does not live long enough
2423
} else {
2524
super let temp = ();
2625
&temp
27-
//~^ ERROR `temp` does not live long enough
2826
});
2927
}

‎tests/ui/borrowck/super-let-in-if-block.stderr‎

Lines changed: 0 additions & 30 deletions
This file was deleted.

‎tests/ui/drop/destructuring-assignments.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@
1414

1515
fn main() {
1616
assert_drop_order(1..=3, |e| {
17-
&({ &raw const *&e.log(1) }, drop(e.log(2)));
17+
&({ &raw const *&e.log(2) }, drop(e.log(1)));
1818
drop(e.log(3));
1919
});
2020
assert_drop_order(1..=3, |e| {
21-
{ let _x; _x = &({ &raw const *&e.log(1) }, drop(e.log(2))); }
21+
{ let _x; _x = &({ &raw const *&e.log(2) }, drop(e.log(1))); }
2222
drop(e.log(3));
2323
});
2424
assert_drop_order(1..=3, |e| {

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ fn should_lint_with_unsafe_block() {
2828
fn should_lint_with_big_block() {
2929
fn f<T>(_: T) {}
3030
f({
31-
&mut || 0
31+
std::convert::identity(&mut || 0)
3232
//~^ ERROR: relative drop order changing
3333
//~| WARN: this changes meaning in Rust 2024
3434
//~| NOTE: this temporary value will be dropped at the end of the block
@@ -40,12 +40,17 @@ fn should_lint_with_big_block() {
4040
fn another_temp_that_is_copy_in_arg() {
4141
fn f() {}
4242
fn g(_: &()) {}
43-
g({ &f() });
43+
g({ std::convert::identity(&f()) });
4444
//~^ ERROR: relative drop order changing
4545
//~| WARN: this changes meaning in Rust 2024
4646
//~| NOTE: this temporary value will be dropped at the end of the block
4747
//~| NOTE: borrow later used by call
4848
//~| NOTE: for more information, see
4949
}
5050

51+
fn do_not_lint_extending_borrow() {
52+
fn f<T>(_: T) {}
53+
f({ &mut || 0 });
54+
}
55+
5156
fn main() {}

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,22 +26,22 @@ LL | f(unsafe { String::new().as_str() }.len());
2626
= note: for more information, see <https://doc.rust-lang.org/edition-guide/rust-2024/temporary-tail-expr-scope.html>
2727

2828
error: relative drop order changing in Rust 2024
29-
--> $DIR/lint-tail-expr-drop-order-borrowck.rs:31:9
29+
--> $DIR/lint-tail-expr-drop-order-borrowck.rs:31:32
3030
|
31-
LL | &mut || 0
32-
| ^^^^^^^^^
33-
| |
34-
| this temporary value will be dropped at the end of the block
31+
LL | std::convert::identity(&mut || 0)
32+
| -----------------------^^^^^^^^^-
33+
| | |
34+
| | this temporary value will be dropped at the end of the block
3535
| borrow later used here
3636
|
3737
= warning: this changes meaning in Rust 2024
3838
= note: for more information, see <https://doc.rust-lang.org/edition-guide/rust-2024/temporary-tail-expr-scope.html>
3939

4040
error: relative drop order changing in Rust 2024
41-
--> $DIR/lint-tail-expr-drop-order-borrowck.rs:43:9
41+
--> $DIR/lint-tail-expr-drop-order-borrowck.rs:43:32
4242
|
43-
LL | g({ &f() });
44-
| - ^^^^ this temporary value will be dropped at the end of the block
43+
LL | g({ std::convert::identity(&f()) });
44+
| - ^^^^ this temporary value will be dropped at the end of the block
4545
| |
4646
| borrow later used by call
4747
|

0 commit comments

Comments
 (0)