Skip to content

Commit bb6b7b6

Browse files
diannedianne
committed
extend borrow operators' operands' temporary scopes
1 parent 2324077 commit bb6b7b6

14 files changed

+147
-218
lines changed

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

Lines changed: 55 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use rustc_hir::intravisit::{self, Visitor};
1515
use rustc_hir::{Arm, Block, Expr, LetStmt, Pat, PatKind, Stmt};
1616
use rustc_index::Idx;
1717
use rustc_middle::middle::region::*;
18+
use rustc_middle::thir::TempLifetime;
1819
use rustc_middle::ty::TyCtxt;
1920
use rustc_session::lint;
2021
use rustc_span::source_map;
@@ -29,7 +30,8 @@ struct Context {
2930
parent: Option<Scope>,
3031

3132
/// Scope of lifetime-extended temporaries. If `None`, extendable expressions have their usual
32-
/// temporary scopes.
33+
/// temporary scopes. Distinguishing the case of non-extended temporaries helps minimize the
34+
/// number of extended lifetimes we record in the [`ScopeTree`]'s `rvalue_scopes` map.
3335
extended_parent: Option<ExtendedScope>,
3436
}
3537

@@ -47,9 +49,32 @@ struct NodeInfo {
4749
extending: bool,
4850
}
4951

50-
/// Scope of lifetime-extended temporaries. If the field is `None`, no drop is scheduled for them.
52+
/// Scope of lifetime-extended temporaries.
5153
#[derive(Debug, Copy, Clone)]
52-
struct ExtendedScope(Option<Scope>);
54+
enum ExtendedScope {
55+
/// Extendable temporaries' scopes will be extended to match the scope of a `let` statement's
56+
/// bindings, a `const`/`static` item, or a `const` block result. In the case of temporaries
57+
/// extended by `const`s and `static`s, the field is `None`, meaning no drop is scheduled.
58+
ThroughDeclaration(Option<Scope>),
59+
/// Extendable temporaries will be dropped in the temporary scope enclosing the given scope.
60+
/// This is a separate variant to minimize calls to [`ScopeTree::default_temporary_scope`].
61+
ThroughExpression(Scope),
62+
}
63+
64+
impl ExtendedScope {
65+
fn to_scope(self, scope_tree: &ScopeTree) -> TempLifetime {
66+
match self {
67+
ExtendedScope::ThroughDeclaration(temp_lifetime) => {
68+
TempLifetime { temp_lifetime, backwards_incompatible: None }
69+
}
70+
ExtendedScope::ThroughExpression(non_extending_parent) => {
71+
let (temp_scope, backwards_incompatible) =
72+
scope_tree.default_temporary_scope(non_extending_parent);
73+
TempLifetime { temp_lifetime: Some(temp_scope), backwards_incompatible }
74+
}
75+
}
76+
}
77+
}
5378

5479
struct ScopeResolutionVisitor<'tcx> {
5580
tcx: TyCtxt<'tcx>,
@@ -178,6 +203,14 @@ fn resolve_block<'tcx>(
178203
.scope_tree
179204
.backwards_incompatible_scope
180205
.insert(local_id, Scope { local_id, data: ScopeData::Node });
206+
207+
// To avoid false positives in `tail_expr_drop_order`, make sure extendable
208+
// temporaries are extended past the block tail even if that doesn't change their
209+
// scopes in the current edition.
210+
if visitor.cx.extended_parent.is_none() {
211+
visitor.cx.extended_parent =
212+
Some(ExtendedScope::ThroughExpression(visitor.cx.parent.unwrap()));
213+
}
181214
}
182215
resolve_expr(visitor, tail_expr, NodeInfo { drop_temps, extending: true });
183216
}
@@ -294,8 +327,9 @@ fn resolve_expr<'tcx>(
294327
// | E& as ...
295328
match expr.kind {
296329
hir::ExprKind::AddrOf(_, _, subexpr) => {
297-
// TODO: generalize
298-
if let Some(ExtendedScope(lifetime)) = visitor.cx.extended_parent {
330+
// Record an extended lifetime for the operand if needed.
331+
if let Some(extended_scope) = visitor.cx.extended_parent {
332+
let lifetime = extended_scope.to_scope(&visitor.scope_tree);
299333
record_subexpr_rvalue_scopes(&mut visitor.scope_tree, subexpr, lifetime);
300334
}
301335
resolve_expr(visitor, subexpr, NodeInfo { drop_temps: false, extending: true });
@@ -561,10 +595,9 @@ fn resolve_local<'tcx>(
561595
// FIXME(super_let): This ignores backward-incompatible drop hints. Implementing BIDs for
562596
// `super let` bindings could improve `tail_expr_drop_order` with regard to `pin!`, etc.
563597

564-
// TODO: generalize
565598
visitor.cx.var_parent = match visitor.cx.extended_parent {
566599
// If the extended parent scope was set, use it.
567-
Some(ExtendedScope(lifetime)) => lifetime,
600+
Some(extended_parent) => extended_parent.to_scope(&visitor.scope_tree).temp_lifetime,
568601
// Otherwise, like a temporaries, bindings are dropped in the enclosing temporary scope.
569602
None => visitor
570603
.cx
@@ -577,7 +610,11 @@ fn resolve_local<'tcx>(
577610
if let Some(pat) = pat
578611
&& is_binding_pat(pat)
579612
{
580-
record_subexpr_rvalue_scopes(&mut visitor.scope_tree, expr, visitor.cx.var_parent);
613+
record_subexpr_rvalue_scopes(
614+
&mut visitor.scope_tree,
615+
expr,
616+
TempLifetime { temp_lifetime: visitor.cx.var_parent, backwards_incompatible: None },
617+
);
581618
}
582619

583620
let prev_extended_parent = visitor.cx.extended_parent;
@@ -586,7 +623,8 @@ fn resolve_local<'tcx>(
586623
// When visiting the initializer, extend borrows and `super let`s accessible through
587624
// extending subexpressions to live in the current variable scope (or in the case of
588625
// statics and consts, for the whole program).
589-
visitor.cx.extended_parent = Some(ExtendedScope(visitor.cx.var_parent));
626+
visitor.cx.extended_parent =
627+
Some(ExtendedScope::ThroughDeclaration(visitor.cx.var_parent));
590628
}
591629

592630
// Make sure we visit the initializer first.
@@ -688,7 +726,7 @@ fn resolve_local<'tcx>(
688726
fn record_subexpr_rvalue_scopes(
689727
scope_tree: &mut ScopeTree,
690728
mut expr: &hir::Expr<'_>,
691-
lifetime: Option<Scope>,
729+
lifetime: TempLifetime,
692730
) {
693731
debug!(?expr, ?lifetime);
694732

@@ -735,6 +773,13 @@ impl<'tcx> ScopeResolutionVisitor<'tcx> {
735773
// account for the destruction scope representing the scope of
736774
// the destructors that run immediately after it completes.
737775
if node_info.drop_temps {
776+
// If this scope corresponds to an extending subexpression, we can extend certain
777+
// temporaries' scopes through it.
778+
if node_info.extending && self.cx.extended_parent.is_none() {
779+
self.cx.extended_parent = Some(ExtendedScope::ThroughExpression(
780+
self.cx.parent.expect("extending subexpressions should have parent scopes"),
781+
));
782+
}
738783
self.enter_scope(Scope { local_id: id, data: ScopeData::Destruction });
739784
}
740785
self.enter_scope(Scope { local_id: id, data: ScopeData::Node });

‎compiler/rustc_middle/src/middle/region.rs‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ pub struct ScopeTree {
225225
/// Tracks the rvalue scoping rules which defines finer scoping for rvalue expressions
226226
/// by applying extended parameter rules.
227227
/// Further details may be found in `rustc_hir_analysis::check::region`.
228-
rvalue_scopes: ItemLocalMap<Option<Scope>>,
228+
rvalue_scopes: ItemLocalMap<TempLifetime>,
229229

230230
/// Backwards incompatible scoping that will be introduced in future editions.
231231
/// This information is used later for linting to identify locals and
@@ -250,12 +250,13 @@ impl ScopeTree {
250250
}
251251

252252
/// Make an association between a sub-expression and an extended lifetime
253-
pub fn record_rvalue_scope(&mut self, var: hir::ItemLocalId, lifetime: Option<Scope>) {
253+
pub fn record_rvalue_scope(&mut self, var: hir::ItemLocalId, lifetime: TempLifetime) {
254254
debug!("record_rvalue_scope(var={var:?}, lifetime={lifetime:?})");
255-
if let Some(lifetime) = lifetime {
255+
if let Some(lifetime) = lifetime.temp_lifetime {
256256
assert!(var != lifetime.local_id);
257257
}
258-
self.rvalue_scopes.insert(var, lifetime);
258+
let old_lifetime = self.rvalue_scopes.insert(var, lifetime);
259+
assert!(old_lifetime.is_none_or(|old| old == lifetime));
259260
}
260261

261262
/// Returns the narrowest scope that encloses `id`, if any.
@@ -335,7 +336,7 @@ impl ScopeTree {
335336
// Check for a designated rvalue scope.
336337
if let Some(&s) = self.rvalue_scopes.get(&expr_id) {
337338
debug!("temporary_scope({expr_id:?}) = {s:?} [custom]");
338-
return TempLifetime { temp_lifetime: s, backwards_incompatible: None };
339+
return s;
339340
}
340341

341342
// Otherwise, locate the innermost terminating scope.

‎compiler/rustc_middle/src/thir.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ pub struct Expr<'tcx> {
265265
}
266266

267267
/// Temporary lifetime information for THIR expressions
268-
#[derive(Clone, Copy, Debug, HashStable)]
268+
#[derive(Clone, Copy, Debug, PartialEq, Eq, HashStable)]
269269
pub struct TempLifetime {
270270
/// Lifetime for temporaries as expected.
271271
/// This should be `None` in a constant context.

‎tests/mir-opt/pre-codegen/slice_index.slice_index_range.PreCodegen.after.panic-abort.mir‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ fn slice_index_range(_1: &[u32], _2: std::ops::Range<usize>) -> &[u32] {
6666
StorageDead(_10);
6767
StorageDead(_9);
6868
_0 = &(*_12);
69-
StorageDead(_12);
7069
StorageDead(_8);
70+
StorageDead(_12);
7171
return;
7272
}
7373

‎tests/mir-opt/pre-codegen/slice_index.slice_index_range.PreCodegen.after.panic-unwind.mir‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,8 @@ fn slice_index_range(_1: &[u32], _2: std::ops::Range<usize>) -> &[u32] {
6666
StorageDead(_10);
6767
StorageDead(_9);
6868
_0 = &(*_12);
69-
StorageDead(_12);
7069
StorageDead(_8);
70+
StorageDead(_12);
7171
return;
7272
}
7373

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

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,5 @@
11
error[E0716]: temporary value dropped while borrowed
2-
--> $DIR/format-args-temporary-scopes.rs:40:90
3-
|
4-
LL | println!("{:?}{:?}", (), match true { true => &"" as &dyn std::fmt::Debug, false => &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
9-
| borrow later used here
10-
|
11-
= note: consider using a `let` binding to create a longer lived value
12-
13-
error[E0716]: temporary value dropped while borrowed
14-
--> $DIR/format-args-temporary-scopes.rs:33:41
15-
|
16-
LL | println!("{:?}{:?}", (), if true { &format!("") } else { "" });
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
21-
| borrow later used here
22-
|
23-
= note: consider using a `let` binding to create a longer lived value
24-
= note: this error originates in the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info)
25-
26-
error[E0716]: temporary value dropped while borrowed
27-
--> $DIR/format-args-temporary-scopes.rs:36:64
2+
--> $DIR/format-args-temporary-scopes.rs:33:64
283
|
294
LL | println!("{:?}{:?}", (), if true { std::convert::identity(&format!("")) } else { "" });
305
| ----------------------------------^^^^^^^^^^^---------------
@@ -36,6 +11,6 @@ LL | println!("{:?}{:?}", (), if true { std::convert::identity(&format!(""))
3611
= note: consider using a `let` binding to create a longer lived value
3712
= note: this error originates in the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info)
3813

39-
error: aborting due to 3 previous errors
14+
error: aborting due to 1 previous error
4015

4116
For more information about this error, try `rustc --explain E0716`.

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

Lines changed: 3 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,3 @@
1-
error[E0716]: temporary value dropped while borrowed
2-
--> $DIR/format-args-temporary-scopes.rs:12:25
3-
|
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
9-
| borrow later used here
10-
|
11-
= note: consider using a `let` binding to create a longer lived value
12-
131
error[E0716]: temporary value dropped while borrowed
142
--> $DIR/format-args-temporary-scopes.rs:17:48
153
|
@@ -23,19 +11,7 @@ LL | println!("{:?}", { std::convert::identity(&temp()) });
2311
= note: consider using a `let` binding to create a longer lived value
2412

2513
error[E0716]: temporary value dropped while borrowed
26-
--> $DIR/format-args-temporary-scopes.rs:23:29
27-
|
28-
LL | println!("{:?}{:?}", { &temp() }, ());
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
33-
| borrow later used here
34-
|
35-
= note: consider using a `let` binding to create a longer lived value
36-
37-
error[E0716]: temporary value dropped while borrowed
38-
--> $DIR/format-args-temporary-scopes.rs:26:52
14+
--> $DIR/format-args-temporary-scopes.rs:24:52
3915
|
4016
LL | println!("{:?}{:?}", { std::convert::identity(&temp()) }, ());
4117
| --------------------------^^^^^^---
@@ -47,32 +23,7 @@ LL | println!("{:?}{:?}", { std::convert::identity(&temp()) }, ());
4723
= note: consider using a `let` binding to create a longer lived value
4824

4925
error[E0716]: temporary value dropped while borrowed
50-
--> $DIR/format-args-temporary-scopes.rs:40:90
51-
|
52-
LL | println!("{:?}{:?}", (), match true { true => &"" as &dyn std::fmt::Debug, false => &temp() });
53-
| ------------------------------------------------------------^^^^^---
54-
| | | |
55-
| | | temporary value is freed at the end of this statement
56-
| | creates a temporary value which is freed while still in use
57-
| borrow later used here
58-
|
59-
= note: consider using a `let` binding to create a longer lived value
60-
61-
error[E0716]: temporary value dropped while borrowed
62-
--> $DIR/format-args-temporary-scopes.rs:33:41
63-
|
64-
LL | println!("{:?}{:?}", (), if true { &format!("") } else { "" });
65-
| -^^^^^^^^^^-
66-
| || |
67-
| || temporary value is freed at the end of this statement
68-
| |creates a temporary value which is freed while still in use
69-
| borrow later used here
70-
|
71-
= note: consider using a `let` binding to create a longer lived value
72-
= note: this error originates in the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info)
73-
74-
error[E0716]: temporary value dropped while borrowed
75-
--> $DIR/format-args-temporary-scopes.rs:36:64
26+
--> $DIR/format-args-temporary-scopes.rs:33:64
7627
|
7728
LL | println!("{:?}{:?}", (), if true { std::convert::identity(&format!("")) } else { "" });
7829
| ------------------------^^^^^^^^^^^-
@@ -84,6 +35,6 @@ LL | println!("{:?}{:?}", (), if true { std::convert::identity(&format!(""))
8435
= note: consider using a `let` binding to create a longer lived value
8536
= note: this error originates in the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info)
8637

87-
error: aborting due to 7 previous errors
38+
error: aborting due to 3 previous errors
8839

8940
For more information about this error, try `rustc --explain E0716`.

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

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,19 @@
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() });
13-
//[e2024]~^ ERROR: temporary value dropped while borrowed [E0716]
1414

1515
// Arguments to function calls aren't extending expressions, so `temp()` is dropped at the end
1616
// of the block in Rust 2024.
1717
println!("{:?}", { std::convert::identity(&temp()) });
1818
//[e2024]~^ ERROR: temporary value dropped while borrowed [E0716]
1919

20-
// In Rust 1.89, `format_args!` extended the lifetime of all extending expressions in its
21-
// arguments when provided with two or more arguments. This caused the result of `temp()` to
22-
// 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.
2322
println!("{:?}{:?}", { &temp() }, ());
24-
//[e2024]~^ ERROR: temporary value dropped while borrowed [E0716]
2523

2624
println!("{:?}{:?}", { std::convert::identity(&temp()) }, ());
2725
//[e2024]~^ ERROR: temporary value dropped while borrowed [E0716]
@@ -31,12 +29,10 @@ fn main() {
3129
// blocks of `if` expressions are temporary scopes in all editions, this affects Rust 2021 and
3230
// earlier as well.
3331
println!("{:?}{:?}", (), if true { &format!("") } else { "" });
34-
//~^ ERROR: temporary value dropped while borrowed [E0716]
3532

3633
println!("{:?}{:?}", (), if true { std::convert::identity(&format!("")) } else { "" });
3734
//~^ ERROR: temporary value dropped while borrowed [E0716]
3835

3936
// This has likewise occurred with `match`, affecting all editions.
4037
println!("{:?}{:?}", (), match true { true => &"" as &dyn std::fmt::Debug, false => &temp() });
41-
//~^ ERROR: temporary value dropped while borrowed [E0716]
4238
}

‎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
}

0 commit comments

Comments
 (0)