Skip to content

Commit f7791f3

Browse files
committed
extend borrow operators' operands' temporary scopes
1 parent 203d86e commit f7791f3

13 files changed

+132
-225
lines changed

compiler/rustc_hir_analysis/src/check/region.rs

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use rustc_hir::{Arm, Block, Expr, LetStmt, Pat, PatKind, Stmt};
1616
use rustc_index::Idx;
1717
use rustc_middle::bug;
1818
use rustc_middle::middle::region::*;
19+
use rustc_middle::thir::TempLifetime;
1920
use rustc_middle::ty::TyCtxt;
2021
use rustc_session::lint;
2122
use rustc_span::source_map;
@@ -52,14 +53,30 @@ enum ExtendedScope {
5253
/// Extendable temporaries' scopes will be extended to match the scope of a `let` statement's
5354
/// bindings, a `const`/`static` item, or a `const` block result.
5455
ThroughDeclaration(Option<Scope>),
55-
/// Lifetime extension isn't possible here.
56-
// TODO: replace this variant
57-
No,
56+
/// Extendable temporaries will be dropped in the temporary scope enclosing the given scope.
57+
/// This is a separate variant to minimize calls to [`ScopeTree::default_temporary_scope`].
58+
ThroughExpression(Scope),
5859
/// The outermost extended scope is set when visiting a body. To make sure we don't forget to do
5960
/// so, this dummy variant is used when creating the [`ScopeResolutionVisitor`].
6061
Unset,
6162
}
6263

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+
ExtendedScope::Unset => bug!("extended parent scope not set"),
76+
}
77+
}
78+
}
79+
6380
struct ScopeResolutionVisitor<'tcx> {
6481
tcx: TyCtxt<'tcx>,
6582

@@ -303,8 +320,10 @@ fn resolve_expr<'tcx>(
303320
// | E& as ...
304321
match expr.kind {
305322
hir::ExprKind::AddrOf(_, _, subexpr) => {
306-
// TODO: generalize
307-
if let ExtendedScope::ThroughDeclaration(lifetime) = visitor.cx.extended_parent {
323+
// Record an extended lifetime if we haven't already done so. This makes sure we don't
324+
// overwrite the scope of e.g. `temp()` in `&*&temp()` when visiting the inner `&`.
325+
if !visitor.scope_tree.has_extended_scope(subexpr.hir_id.local_id) {
326+
let lifetime = visitor.cx.extended_parent.to_scope(&visitor.scope_tree);
308327
record_projection_rvalue_scopes(&mut visitor.scope_tree, subexpr, lifetime);
309328
}
310329
resolve_expr(visitor, subexpr, ScopeInfo { terminating: false, extending: true });
@@ -578,22 +597,19 @@ fn resolve_local<'tcx>(
578597
// FIXME(super_let): This ignores backward-incompatible drop hints. Implementing BIDs for
579598
// `super let` bindings could improve `tail_expr_drop_order` with regard to `pin!`, etc.
580599

581-
// TODO: generalize
582-
visitor.cx.var_parent = match visitor.cx.extended_parent {
583-
ExtendedScope::ThroughDeclaration(lifetime) => lifetime,
584-
ExtendedScope::No => visitor
585-
.cx
586-
.var_parent
587-
.map(|block| visitor.scope_tree.default_temporary_scope(block).0),
588-
ExtendedScope::Unset => bug!("extended parent scope not set"),
589-
}
600+
visitor.cx.var_parent =
601+
visitor.cx.extended_parent.to_scope(&visitor.scope_tree).temp_lifetime;
590602
}
591603

592604
if let Some(expr) = init {
593605
if let Some(pat) = pat
594606
&& is_binding_pat(pat)
595607
{
596-
record_projection_rvalue_scopes(&mut visitor.scope_tree, expr, visitor.cx.var_parent);
608+
record_projection_rvalue_scopes(
609+
&mut visitor.scope_tree,
610+
expr,
611+
TempLifetime { temp_lifetime: visitor.cx.var_parent, backwards_incompatible: None },
612+
);
597613
}
598614

599615
let prev_extended_parent = visitor.cx.extended_parent;
@@ -704,7 +720,7 @@ fn resolve_local<'tcx>(
704720
fn record_projection_rvalue_scopes(
705721
scope_tree: &mut ScopeTree,
706722
mut expr: &hir::Expr<'_>,
707-
lifetime: Option<Scope>,
723+
lifetime: TempLifetime,
708724
) {
709725
debug!(?expr, ?lifetime);
710726

@@ -754,9 +770,11 @@ impl<'tcx> ScopeResolutionVisitor<'tcx> {
754770
self.enter_scope(Scope { local_id: id, data: ScopeData::Destruction });
755771
}
756772
self.enter_scope(Scope { local_id: id, data: ScopeData::Node });
757-
// TODO: set to the enclosing temporary scope rather than `No`
773+
// If this scope corresponds to a non-extending subexpression, limit the scopes of
774+
// temporaries to the enclosing temporary scope.
758775
if !scope_info.extending {
759-
self.cx.extended_parent = ExtendedScope::No;
776+
self.cx.extended_parent =
777+
ExtendedScope::ThroughExpression(Scope { local_id: id, data: ScopeData::Node });
760778
}
761779
}
762780

compiler/rustc_middle/src/middle/region.rs

Lines changed: 11 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,18 @@ 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 result = self.rvalue_scopes.try_insert(var, lifetime);
259+
assert!(result.is_ok());
260+
}
261+
262+
/// Check for an association between a sub-expression and an extended lifetime
263+
pub fn has_extended_scope(&self, var: hir::ItemLocalId) -> bool {
264+
self.rvalue_scopes.contains_key(&var)
259265
}
260266

261267
/// Returns the narrowest scope that encloses `id`, if any.
@@ -335,7 +341,7 @@ impl ScopeTree {
335341
// Check for a designated rvalue scope.
336342
if let Some(&s) = self.rvalue_scopes.get(&expr_id) {
337343
debug!("temporary_scope({expr_id:?}) = {s:?} [custom]");
338-
return TempLifetime { temp_lifetime: s, backwards_incompatible: None };
344+
return s;
339345
}
340346

341347
// Otherwise, locate the innermost terminating scope.

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

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`.
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
}

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

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

0 commit comments

Comments
 (0)