Skip to content

Commit 2ecbcc5

Browse files
committed
warn on shortening super let binding lifetimes
1 parent 74ab7bd commit 2ecbcc5

File tree

6 files changed

+80
-31
lines changed

6 files changed

+80
-31
lines changed

compiler/rustc_hir_analysis/src/check/region.rs

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use tracing::debug;
2424
#[derive(Debug, Copy, Clone)]
2525
struct Context {
2626
/// The scope that contains any new variables declared.
27-
var_parent: Option<Scope>,
27+
var_parent: (Option<Scope>, ScopeCompatibility),
2828

2929
/// Region parent of expressions, etc.
3030
parent: Option<Scope>,
@@ -56,14 +56,18 @@ struct ExtendedTemporaryScope {
5656

5757
/// Records the lifetime of a local variable as `cx.var_parent`
5858
fn record_var_lifetime(visitor: &mut ScopeResolutionVisitor<'_>, var_id: hir::ItemLocalId) {
59-
match visitor.cx.var_parent {
59+
let (var_parent_scope, var_parent_compat) = visitor.cx.var_parent;
60+
match var_parent_scope {
6061
None => {
6162
// this can happen in extern fn declarations like
6263
//
6364
// extern fn isalnum(c: c_int) -> c_int
6465
}
6566
Some(parent_scope) => visitor.scope_tree.record_var_scope(var_id, parent_scope),
6667
}
68+
if let ScopeCompatibility::FutureIncompatible { shortens_to } = var_parent_compat {
69+
visitor.scope_tree.record_future_incompatible_var_scope(var_id, shortens_to);
70+
}
6771
}
6872

6973
fn resolve_block<'tcx>(
@@ -101,7 +105,7 @@ fn resolve_block<'tcx>(
101105
// itself has returned.
102106

103107
visitor.enter_node_scope_with_dtor(blk.hir_id.local_id, terminating);
104-
visitor.cx.var_parent = visitor.cx.parent;
108+
visitor.cx.var_parent = (visitor.cx.parent, ScopeCompatibility::FutureCompatible);
105109

106110
{
107111
// This block should be kept approximately in sync with
@@ -120,7 +124,8 @@ fn resolve_block<'tcx>(
120124
local_id: blk.hir_id.local_id,
121125
data: ScopeData::Remainder(FirstStatementIndex::new(i)),
122126
});
123-
visitor.cx.var_parent = visitor.cx.parent;
127+
visitor.cx.var_parent =
128+
(visitor.cx.parent, ScopeCompatibility::FutureCompatible);
124129
visitor.visit_stmt(statement);
125130
// We need to back out temporarily to the last enclosing scope
126131
// for the `else` block, so that even the temporaries receiving
@@ -144,7 +149,8 @@ fn resolve_block<'tcx>(
144149
local_id: blk.hir_id.local_id,
145150
data: ScopeData::Remainder(FirstStatementIndex::new(i)),
146151
});
147-
visitor.cx.var_parent = visitor.cx.parent;
152+
visitor.cx.var_parent =
153+
(visitor.cx.parent, ScopeCompatibility::FutureCompatible);
148154
visitor.visit_stmt(statement)
149155
}
150156
hir::StmtKind::Item(..) => {
@@ -208,15 +214,15 @@ fn resolve_arm<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, arm: &'tcx hir:
208214
let prev_cx = visitor.cx;
209215

210216
visitor.enter_node_scope_with_dtor(arm.hir_id.local_id, true);
211-
visitor.cx.var_parent = visitor.cx.parent;
217+
visitor.cx.var_parent = (visitor.cx.parent, ScopeCompatibility::FutureCompatible);
212218

213219
resolve_pat(visitor, arm.pat);
214220
if let Some(guard) = arm.guard {
215221
// We introduce a new scope to contain bindings and temporaries from `if let` guards, to
216222
// ensure they're dropped before the arm's pattern's bindings. This extends to the end of
217223
// the arm body and is the scope of its locals as well.
218224
visitor.enter_scope(Scope { local_id: arm.hir_id.local_id, data: ScopeData::MatchGuard });
219-
visitor.cx.var_parent = visitor.cx.parent;
225+
visitor.cx.var_parent = (visitor.cx.parent, ScopeCompatibility::FutureCompatible);
220226
resolve_cond(visitor, guard);
221227
}
222228
resolve_expr(visitor, arm.body, false);
@@ -373,7 +379,7 @@ fn resolve_expr<'tcx>(
373379
ScopeData::IfThen
374380
};
375381
visitor.enter_scope(Scope { local_id: then.hir_id.local_id, data });
376-
visitor.cx.var_parent = visitor.cx.parent;
382+
visitor.cx.var_parent = (visitor.cx.parent, ScopeCompatibility::FutureCompatible);
377383
resolve_cond(visitor, cond);
378384
resolve_expr(visitor, then, true);
379385
visitor.cx = expr_cx;
@@ -388,7 +394,7 @@ fn resolve_expr<'tcx>(
388394
ScopeData::IfThen
389395
};
390396
visitor.enter_scope(Scope { local_id: then.hir_id.local_id, data });
391-
visitor.cx.var_parent = visitor.cx.parent;
397+
visitor.cx.var_parent = (visitor.cx.parent, ScopeCompatibility::FutureCompatible);
392398
resolve_cond(visitor, cond);
393399
resolve_expr(visitor, then, true);
394400
visitor.cx = expr_cx;
@@ -498,7 +504,7 @@ fn resolve_local<'tcx>(
498504
//
499505
// Processing of `let a` will have already decided to extend the lifetime of this
500506
// `super let` to its own var_scope. We use that scope.
501-
visitor.cx.var_parent = scope.scope;
507+
visitor.cx.var_parent = (scope.scope, scope.compat);
502508
// Inherit compatibility from the original `let` statement. If the original `let`
503509
// was regular, lifetime extension should apply as normal. If the original `let` was
504510
// `super`, blocks within the initializer will be affected by #145838.
@@ -512,9 +518,11 @@ fn resolve_local<'tcx>(
512518
//
513519
// Iterate up to the enclosing destruction scope to find the same scope that will also
514520
// be used for the result of the block itself.
515-
if let Some(inner_scope) = visitor.cx.var_parent {
516-
(visitor.cx.var_parent, _) =
517-
visitor.scope_tree.default_temporary_scope(inner_scope)
521+
if let (Some(inner_scope), _) = visitor.cx.var_parent {
522+
// NB(@dianne): This could use the incompatibility reported by
523+
// `default_temporary_scope` to make `tail_expr_drop_order` more comprehensive.
524+
visitor.cx.var_parent =
525+
(visitor.scope_tree.default_temporary_scope(inner_scope).0, ScopeCompatibility::FutureCompatible);
518526
}
519527
// Blocks within the initializer will be affected by #145838.
520528
(LetKind::Super, ScopeCompatibility::FutureCompatible)
@@ -524,7 +532,7 @@ fn resolve_local<'tcx>(
524532

525533
if let Some(expr) = init {
526534
let scope = ExtendedTemporaryScope {
527-
scope: visitor.cx.var_parent,
535+
scope: visitor.cx.var_parent.0,
528536
let_kind: source_let_kind,
529537
compat,
530538
};
@@ -536,8 +544,8 @@ fn resolve_local<'tcx>(
536544
expr.hir_id,
537545
RvalueCandidate {
538546
target: expr.hir_id.local_id,
539-
lifetime: visitor.cx.var_parent,
540-
compat: ScopeCompatibility::FutureCompatible,
547+
lifetime: visitor.cx.var_parent.0,
548+
compat: visitor.cx.var_parent.1,
541549
},
542550
);
543551
}
@@ -818,7 +826,7 @@ impl<'tcx> Visitor<'tcx> for ScopeResolutionVisitor<'tcx> {
818826
self.enter_body(body.value.hir_id, |this| {
819827
if this.tcx.hir_body_owner_kind(owner_id).is_fn_or_closure() {
820828
// The arguments and `self` are parented to the fn.
821-
this.cx.var_parent = this.cx.parent;
829+
this.cx.var_parent = (this.cx.parent, ScopeCompatibility::FutureCompatible);
822830
for param in body.params {
823831
this.visit_pat(param.pat);
824832
}
@@ -844,7 +852,7 @@ impl<'tcx> Visitor<'tcx> for ScopeResolutionVisitor<'tcx> {
844852
// would *not* let the `f()` temporary escape into an outer scope
845853
// (i.e., `'static`), which means that after `g` returns, it drops,
846854
// and all the associated destruction scope rules apply.
847-
this.cx.var_parent = None;
855+
this.cx.var_parent = (None, ScopeCompatibility::FutureCompatible);
848856
this.enter_scope(Scope {
849857
local_id: body.value.hir_id.local_id,
850858
data: ScopeData::Destruction,
@@ -896,7 +904,7 @@ pub(crate) fn region_scope_tree(tcx: TyCtxt<'_>, def_id: DefId) -> &ScopeTree {
896904
let mut visitor = ScopeResolutionVisitor {
897905
tcx,
898906
scope_tree: ScopeTree::default(),
899-
cx: Context { parent: None, var_parent: None },
907+
cx: Context { parent: None, var_parent: (None, ScopeCompatibility::FutureCompatible) },
900908
extended_super_lets: Default::default(),
901909
};
902910

compiler/rustc_hir_typeck/src/method/suggest.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -441,8 +441,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
441441
// Check scope of binding.
442442
fn is_sub_scope(&self, sub_id: hir::ItemLocalId, super_id: hir::ItemLocalId) -> bool {
443443
let scope_tree = self.fcx.tcx.region_scope_tree(self.fcx.body_id);
444-
if let Some(sub_var_scope) = scope_tree.var_scope(sub_id)
445-
&& let Some(super_var_scope) = scope_tree.var_scope(super_id)
444+
if let (Some(sub_var_scope), _) = scope_tree.var_scope(sub_id)
445+
&& let (Some(super_var_scope), _) = scope_tree.var_scope(super_id)
446446
&& scope_tree.is_subscope_of(sub_var_scope, super_var_scope)
447447
{
448448
return true;

compiler/rustc_middle/src/middle/region.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,10 @@ pub struct ScopeTree {
222222
/// variable is declared.
223223
var_map: FxIndexMap<hir::ItemLocalId, Scope>,
224224

225+
/// Maps from bindings to their future scopes after #145838 for the
226+
/// `macro_extended_temporary_scopes` lint.
227+
var_compatibility_map: FxIndexMap<hir::ItemLocalId, Scope>,
228+
225229
/// Identifies expressions which, if captured into a temporary, ought to
226230
/// have a temporary whose lifetime extends to the end of the enclosing *block*,
227231
/// and not the enclosing *statement*. Expressions that are not present in this
@@ -274,6 +278,11 @@ impl ScopeTree {
274278
self.var_map.insert(var, lifetime);
275279
}
276280

281+
pub fn record_future_incompatible_var_scope(&mut self, var: hir::ItemLocalId, lifetime: Scope) {
282+
assert!(var != lifetime.local_id);
283+
self.var_compatibility_map.insert(var, lifetime);
284+
}
285+
277286
pub fn record_rvalue_candidate(&mut self, var: HirId, candidate: RvalueCandidate) {
278287
debug!("record_rvalue_candidate(var={var:?}, candidate={candidate:?})");
279288
if let Some(lifetime) = &candidate.lifetime {
@@ -287,9 +296,14 @@ impl ScopeTree {
287296
self.parent_map.get(&id).cloned()
288297
}
289298

290-
/// Returns the lifetime of the local variable `var_id`, if any.
291-
pub fn var_scope(&self, var_id: hir::ItemLocalId) -> Option<Scope> {
292-
self.var_map.get(&var_id).cloned()
299+
/// Returns the lifetime of the local variable `var_id`, if any, as well as whether it is
300+
/// shortening after #145838.
301+
pub fn var_scope(&self, var_id: hir::ItemLocalId) -> (Option<Scope>, ScopeCompatibility) {
302+
let compat = match self.var_compatibility_map.get(&var_id) {
303+
Some(&shortens_to) => ScopeCompatibility::FutureIncompatible { shortens_to },
304+
None => ScopeCompatibility::FutureCompatible,
305+
};
306+
(self.var_map.get(&var_id).cloned(), compat)
293307
}
294308

295309
/// Returns `true` if `subscope` is equal to or is lexically nested inside `superscope`, and

compiler/rustc_mir_build/src/builder/matches/mod.rs

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use rustc_data_structures::fx::FxIndexMap;
1515
use rustc_data_structures::stack::ensure_sufficient_stack;
1616
use rustc_hir::{BindingMode, ByRef, LetStmt, LocalSource, Node};
1717
use rustc_middle::bug;
18-
use rustc_middle::middle::region;
18+
use rustc_middle::middle::region::{self, ScopeCompatibility};
1919
use rustc_middle::mir::*;
2020
use rustc_middle::thir::{self, *};
2121
use rustc_middle::ty::{self, CanonicalUserTypeAnnotation, Ty, ValTree, ValTreeKind};
@@ -807,10 +807,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
807807
self.cfg.push(block, Statement::new(source_info, StatementKind::StorageLive(local_id)));
808808
// Although there is almost always scope for given variable in corner cases
809809
// like #92893 we might get variable with no scope.
810-
if let Some(region_scope) = self.region_scope_tree.var_scope(var.0.local_id)
811-
&& matches!(schedule_drop, ScheduleDrops::Yes)
812-
{
813-
self.schedule_drop(span, region_scope, local_id, DropKind::Storage);
810+
if matches!(schedule_drop, ScheduleDrops::Yes) {
811+
let (var_scope, var_scope_compat) = self.region_scope_tree.var_scope(var.0.local_id);
812+
if let Some(region_scope) = var_scope {
813+
self.schedule_drop(span, region_scope, local_id, DropKind::Storage);
814+
}
815+
if let ScopeCompatibility::FutureIncompatible { shortens_to } = var_scope_compat {
816+
self.schedule_backwards_incompatible_drop(
817+
span,
818+
shortens_to,
819+
local_id,
820+
BackwardIncompatibleDropReason::MacroExtendedScope,
821+
);
822+
}
814823
}
815824
Place::from(local_id)
816825
}
@@ -822,7 +831,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
822831
for_guard: ForGuard,
823832
) {
824833
let local_id = self.var_local_id(var, for_guard);
825-
if let Some(region_scope) = self.region_scope_tree.var_scope(var.0.local_id) {
834+
// We can ignore the var scope's future-compatibility information since we've already taken
835+
// it into account when scheduling the storage drop in `storage_live_binding`.
836+
if let (Some(region_scope), _) = self.region_scope_tree.var_scope(var.0.local_id) {
826837
self.schedule_drop(span, region_scope, local_id, DropKind::Value);
827838
}
828839
}

tests/ui/lifetimes/lint-macro-extended-temporary-scopes/extended-super-let-bindings.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,6 @@ fn main() {
77
// The `()` argument to the inner `format_args!` is promoted, but the lifetimes of the internal
88
// `super let` temporaries in its expansion shorten, making this an error in Rust 1.92.
99
println!("{:?}{}", (), { format_args!("{:?}", ()) });
10-
// TODO: warn
10+
//~^ WARN temporary lifetime shortening in Rust 1.92
11+
//~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
1112
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
warning: temporary lifetime shortening in Rust 1.92
2+
--> $DIR/extended-super-let-bindings.rs:9:30
3+
|
4+
LL | println!("{:?}{}", (), { format_args!("{:?}", ()) });
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^ - ...which will be dropped at the end of this block in Rust 1.92
6+
| |
7+
| this expression creates a temporary value...
8+
|
9+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
10+
= note: for more information, see <https://doc.rust-lang.org/rustc/lints/listing/warn-by-default.html#macro-extended-temporary-scopes>
11+
= note: consider using a `let` binding to create a longer lived value
12+
= note: `#[warn(macro_extended_temporary_scopes)]` (part of `#[warn(future_incompatible)]`) on by default
13+
14+
warning: 1 warning emitted
15+

0 commit comments

Comments
 (0)