From 164620feda080e2a88cf0e503b4f12e446e339e1 Mon Sep 17 00:00:00 2001 From: dianne Date: Mon, 29 Sep 2025 00:02:14 -0700 Subject: [PATCH] warn on shortening `super let` binding lifetimes --- .../rustc_hir_analysis/src/check/region.rs | 46 +++++++++++-------- .../rustc_hir_typeck/src/method/suggest.rs | 4 +- compiler/rustc_middle/src/middle/region.rs | 20 ++++++-- .../src/builder/matches/mod.rs | 23 +++++++--- .../src/loops/needless_range_loop.rs | 3 +- src/tools/clippy/clippy_lints/src/shadow.rs | 4 +- .../extended-super-let-bindings.rs | 3 +- .../extended-super-let-bindings.stderr | 15 ++++++ 8 files changed, 84 insertions(+), 34 deletions(-) create mode 100644 tests/ui/lifetimes/lint-macro-extended-temporary-scopes/extended-super-let-bindings.stderr diff --git a/compiler/rustc_hir_analysis/src/check/region.rs b/compiler/rustc_hir_analysis/src/check/region.rs index 294386c3531b..eb5ce80cc0ea 100644 --- a/compiler/rustc_hir_analysis/src/check/region.rs +++ b/compiler/rustc_hir_analysis/src/check/region.rs @@ -24,7 +24,7 @@ use tracing::debug; #[derive(Debug, Copy, Clone)] struct Context { /// The scope that contains any new variables declared. - var_parent: Option, + var_parent: (Option, ScopeCompatibility), /// Region parent of expressions, etc. parent: Option, @@ -56,7 +56,8 @@ struct ExtendedTemporaryScope { /// Records the lifetime of a local variable as `cx.var_parent` fn record_var_lifetime(visitor: &mut ScopeResolutionVisitor<'_>, var_id: hir::ItemLocalId) { - match visitor.cx.var_parent { + let (var_parent_scope, var_parent_compat) = visitor.cx.var_parent; + match var_parent_scope { None => { // this can happen in extern fn declarations like // @@ -64,6 +65,9 @@ fn record_var_lifetime(visitor: &mut ScopeResolutionVisitor<'_>, var_id: hir::It } Some(parent_scope) => visitor.scope_tree.record_var_scope(var_id, parent_scope), } + if let ScopeCompatibility::FutureIncompatible { shortens_to } = var_parent_compat { + visitor.scope_tree.record_future_incompatible_var_scope(var_id, shortens_to); + } } fn resolve_block<'tcx>( @@ -101,7 +105,7 @@ fn resolve_block<'tcx>( // itself has returned. visitor.enter_node_scope_with_dtor(blk.hir_id.local_id, terminating); - visitor.cx.var_parent = visitor.cx.parent; + visitor.cx.var_parent = (visitor.cx.parent, ScopeCompatibility::FutureCompatible); { // This block should be kept approximately in sync with @@ -120,7 +124,8 @@ fn resolve_block<'tcx>( local_id: blk.hir_id.local_id, data: ScopeData::Remainder(FirstStatementIndex::new(i)), }); - visitor.cx.var_parent = visitor.cx.parent; + visitor.cx.var_parent = + (visitor.cx.parent, ScopeCompatibility::FutureCompatible); visitor.visit_stmt(statement); // We need to back out temporarily to the last enclosing scope // for the `else` block, so that even the temporaries receiving @@ -144,7 +149,8 @@ fn resolve_block<'tcx>( local_id: blk.hir_id.local_id, data: ScopeData::Remainder(FirstStatementIndex::new(i)), }); - visitor.cx.var_parent = visitor.cx.parent; + visitor.cx.var_parent = + (visitor.cx.parent, ScopeCompatibility::FutureCompatible); visitor.visit_stmt(statement) } hir::StmtKind::Item(..) => { @@ -208,7 +214,7 @@ fn resolve_arm<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, arm: &'tcx hir: let prev_cx = visitor.cx; visitor.enter_node_scope_with_dtor(arm.hir_id.local_id, true); - visitor.cx.var_parent = visitor.cx.parent; + visitor.cx.var_parent = (visitor.cx.parent, ScopeCompatibility::FutureCompatible); resolve_pat(visitor, arm.pat); if let Some(guard) = arm.guard { @@ -216,7 +222,7 @@ fn resolve_arm<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, arm: &'tcx hir: // ensure they're dropped before the arm's pattern's bindings. This extends to the end of // the arm body and is the scope of its locals as well. visitor.enter_scope(Scope { local_id: arm.hir_id.local_id, data: ScopeData::MatchGuard }); - visitor.cx.var_parent = visitor.cx.parent; + visitor.cx.var_parent = (visitor.cx.parent, ScopeCompatibility::FutureCompatible); resolve_cond(visitor, guard); } resolve_expr(visitor, arm.body, false); @@ -373,7 +379,7 @@ fn resolve_expr<'tcx>( ScopeData::IfThen }; visitor.enter_scope(Scope { local_id: then.hir_id.local_id, data }); - visitor.cx.var_parent = visitor.cx.parent; + visitor.cx.var_parent = (visitor.cx.parent, ScopeCompatibility::FutureCompatible); resolve_cond(visitor, cond); resolve_expr(visitor, then, true); visitor.cx = expr_cx; @@ -388,7 +394,7 @@ fn resolve_expr<'tcx>( ScopeData::IfThen }; visitor.enter_scope(Scope { local_id: then.hir_id.local_id, data }); - visitor.cx.var_parent = visitor.cx.parent; + visitor.cx.var_parent = (visitor.cx.parent, ScopeCompatibility::FutureCompatible); resolve_cond(visitor, cond); resolve_expr(visitor, then, true); visitor.cx = expr_cx; @@ -498,7 +504,7 @@ fn resolve_local<'tcx>( // // Processing of `let a` will have already decided to extend the lifetime of this // `super let` to its own var_scope. We use that scope. - visitor.cx.var_parent = scope.scope; + visitor.cx.var_parent = (scope.scope, scope.compat); // Inherit compatibility from the original `let` statement. If the original `let` // was regular, lifetime extension should apply as normal. If the original `let` was // `super`, blocks within the initializer will be affected by #145838. @@ -512,9 +518,11 @@ fn resolve_local<'tcx>( // // Iterate up to the enclosing destruction scope to find the same scope that will also // be used for the result of the block itself. - if let Some(inner_scope) = visitor.cx.var_parent { - (visitor.cx.var_parent, _) = - visitor.scope_tree.default_temporary_scope(inner_scope) + if let (Some(inner_scope), _) = visitor.cx.var_parent { + // NB(@dianne): This could use the incompatibility reported by + // `default_temporary_scope` to make `tail_expr_drop_order` more comprehensive. + visitor.cx.var_parent = + (visitor.scope_tree.default_temporary_scope(inner_scope).0, ScopeCompatibility::FutureCompatible); } // Blocks within the initializer will be affected by #145838. (LetKind::Super, ScopeCompatibility::FutureCompatible) @@ -524,7 +532,7 @@ fn resolve_local<'tcx>( if let Some(expr) = init { let scope = ExtendedTemporaryScope { - scope: visitor.cx.var_parent, + scope: visitor.cx.var_parent.0, let_kind: source_let_kind, compat, }; @@ -536,8 +544,8 @@ fn resolve_local<'tcx>( expr.hir_id, RvalueCandidate { target: expr.hir_id.local_id, - lifetime: visitor.cx.var_parent, - compat: ScopeCompatibility::FutureCompatible, + lifetime: visitor.cx.var_parent.0, + compat: visitor.cx.var_parent.1, }, ); } @@ -818,7 +826,7 @@ impl<'tcx> Visitor<'tcx> for ScopeResolutionVisitor<'tcx> { self.enter_body(body.value.hir_id, |this| { if this.tcx.hir_body_owner_kind(owner_id).is_fn_or_closure() { // The arguments and `self` are parented to the fn. - this.cx.var_parent = this.cx.parent; + this.cx.var_parent = (this.cx.parent, ScopeCompatibility::FutureCompatible); for param in body.params { this.visit_pat(param.pat); } @@ -844,7 +852,7 @@ impl<'tcx> Visitor<'tcx> for ScopeResolutionVisitor<'tcx> { // would *not* let the `f()` temporary escape into an outer scope // (i.e., `'static`), which means that after `g` returns, it drops, // and all the associated destruction scope rules apply. - this.cx.var_parent = None; + this.cx.var_parent = (None, ScopeCompatibility::FutureCompatible); this.enter_scope(Scope { local_id: body.value.hir_id.local_id, data: ScopeData::Destruction, @@ -896,7 +904,7 @@ pub(crate) fn region_scope_tree(tcx: TyCtxt<'_>, def_id: DefId) -> &ScopeTree { let mut visitor = ScopeResolutionVisitor { tcx, scope_tree: ScopeTree::default(), - cx: Context { parent: None, var_parent: None }, + cx: Context { parent: None, var_parent: (None, ScopeCompatibility::FutureCompatible) }, extended_super_lets: Default::default(), }; diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index c8f6c06b720d..559f354a3a7c 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -441,8 +441,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Check scope of binding. fn is_sub_scope(&self, sub_id: hir::ItemLocalId, super_id: hir::ItemLocalId) -> bool { let scope_tree = self.fcx.tcx.region_scope_tree(self.fcx.body_id); - if let Some(sub_var_scope) = scope_tree.var_scope(sub_id) - && let Some(super_var_scope) = scope_tree.var_scope(super_id) + if let (Some(sub_var_scope), _) = scope_tree.var_scope(sub_id) + && let (Some(super_var_scope), _) = scope_tree.var_scope(super_id) && scope_tree.is_subscope_of(sub_var_scope, super_var_scope) { return true; diff --git a/compiler/rustc_middle/src/middle/region.rs b/compiler/rustc_middle/src/middle/region.rs index bd8f06c30ecd..0c881bc3a3b5 100644 --- a/compiler/rustc_middle/src/middle/region.rs +++ b/compiler/rustc_middle/src/middle/region.rs @@ -222,6 +222,10 @@ pub struct ScopeTree { /// variable is declared. var_map: FxIndexMap, + /// Maps from bindings to their future scopes after #145838 for the + /// `macro_extended_temporary_scopes` lint. + var_compatibility_map: FxIndexMap, + /// Identifies expressions which, if captured into a temporary, ought to /// have a temporary whose lifetime extends to the end of the enclosing *block*, /// and not the enclosing *statement*. Expressions that are not present in this @@ -274,6 +278,11 @@ impl ScopeTree { self.var_map.insert(var, lifetime); } + pub fn record_future_incompatible_var_scope(&mut self, var: hir::ItemLocalId, lifetime: Scope) { + assert!(var != lifetime.local_id); + self.var_compatibility_map.insert(var, lifetime); + } + pub fn record_rvalue_candidate(&mut self, var: HirId, candidate: RvalueCandidate) { debug!("record_rvalue_candidate(var={var:?}, candidate={candidate:?})"); if let Some(lifetime) = &candidate.lifetime { @@ -287,9 +296,14 @@ impl ScopeTree { self.parent_map.get(&id).cloned() } - /// Returns the lifetime of the local variable `var_id`, if any. - pub fn var_scope(&self, var_id: hir::ItemLocalId) -> Option { - self.var_map.get(&var_id).cloned() + /// Returns the lifetime of the local variable `var_id`, if any, as well as whether it is + /// shortening after #145838. + pub fn var_scope(&self, var_id: hir::ItemLocalId) -> (Option, ScopeCompatibility) { + let compat = match self.var_compatibility_map.get(&var_id) { + Some(&shortens_to) => ScopeCompatibility::FutureIncompatible { shortens_to }, + None => ScopeCompatibility::FutureCompatible, + }; + (self.var_map.get(&var_id).cloned(), compat) } /// Returns `true` if `subscope` is equal to or is lexically nested inside `superscope`, and diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index d216c4ecd115..9ba2f4db4f03 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -15,7 +15,7 @@ use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_hir::{BindingMode, ByRef, LetStmt, LocalSource, Node}; use rustc_middle::bug; -use rustc_middle::middle::region; +use rustc_middle::middle::region::{self, ScopeCompatibility}; use rustc_middle::mir::*; use rustc_middle::thir::{self, *}; use rustc_middle::ty::{self, CanonicalUserTypeAnnotation, Ty, ValTree, ValTreeKind}; @@ -807,10 +807,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.cfg.push(block, Statement::new(source_info, StatementKind::StorageLive(local_id))); // Although there is almost always scope for given variable in corner cases // like #92893 we might get variable with no scope. - if let Some(region_scope) = self.region_scope_tree.var_scope(var.0.local_id) - && matches!(schedule_drop, ScheduleDrops::Yes) - { - self.schedule_drop(span, region_scope, local_id, DropKind::Storage); + if matches!(schedule_drop, ScheduleDrops::Yes) { + let (var_scope, var_scope_compat) = self.region_scope_tree.var_scope(var.0.local_id); + if let Some(region_scope) = var_scope { + self.schedule_drop(span, region_scope, local_id, DropKind::Storage); + } + if let ScopeCompatibility::FutureIncompatible { shortens_to } = var_scope_compat { + self.schedule_backwards_incompatible_drop( + span, + shortens_to, + local_id, + BackwardIncompatibleDropReason::MacroExtendedScope, + ); + } } Place::from(local_id) } @@ -822,7 +831,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { for_guard: ForGuard, ) { let local_id = self.var_local_id(var, for_guard); - if let Some(region_scope) = self.region_scope_tree.var_scope(var.0.local_id) { + // We can ignore the var scope's future-compatibility information since we've already taken + // it into account when scheduling the storage drop in `storage_live_binding`. + if let (Some(region_scope), _) = self.region_scope_tree.var_scope(var.0.local_id) { self.schedule_drop(span, region_scope, local_id, DropKind::Value); } } diff --git a/src/tools/clippy/clippy_lints/src/loops/needless_range_loop.rs b/src/tools/clippy/clippy_lints/src/loops/needless_range_loop.rs index 11edb929d70b..de162a12e47d 100644 --- a/src/tools/clippy/clippy_lints/src/loops/needless_range_loop.rs +++ b/src/tools/clippy/clippy_lints/src/loops/needless_range_loop.rs @@ -64,7 +64,7 @@ pub(super) fn check<'tcx>( if let Some(indexed_extent) = indexed_extent { let parent_def_id = cx.tcx.hir_get_parent_item(expr.hir_id); let region_scope_tree = cx.tcx.region_scope_tree(parent_def_id); - let pat_extent = region_scope_tree.var_scope(pat.hir_id.local_id).unwrap(); + let pat_extent = region_scope_tree.var_scope(pat.hir_id.local_id).0.unwrap(); if region_scope_tree.is_subscope_of(indexed_extent, pat_extent) { return; } @@ -298,6 +298,7 @@ impl<'tcx> VarVisitor<'_, 'tcx> { .tcx .region_scope_tree(parent_def_id) .var_scope(hir_id.local_id) + .0 .unwrap(); if index_used_directly { self.indexed_directly.insert( diff --git a/src/tools/clippy/clippy_lints/src/shadow.rs b/src/tools/clippy/clippy_lints/src/shadow.rs index 14399867f318..ee80e75a9d1f 100644 --- a/src/tools/clippy/clippy_lints/src/shadow.rs +++ b/src/tools/clippy/clippy_lints/src/shadow.rs @@ -180,8 +180,8 @@ impl<'tcx> LateLintPass<'tcx> for Shadow { fn is_shadow(cx: &LateContext<'_>, owner: LocalDefId, first: ItemLocalId, second: ItemLocalId) -> bool { let scope_tree = cx.tcx.region_scope_tree(owner.to_def_id()); - if let Some(first_scope) = scope_tree.var_scope(first) - && let Some(second_scope) = scope_tree.var_scope(second) + if let Some(first_scope) = scope_tree.var_scope(first).0 + && let Some(second_scope) = scope_tree.var_scope(second).0 { return scope_tree.is_subscope_of(second_scope, first_scope); } diff --git a/tests/ui/lifetimes/lint-macro-extended-temporary-scopes/extended-super-let-bindings.rs b/tests/ui/lifetimes/lint-macro-extended-temporary-scopes/extended-super-let-bindings.rs index 950d1a4eaaa9..c3f407633dcb 100644 --- a/tests/ui/lifetimes/lint-macro-extended-temporary-scopes/extended-super-let-bindings.rs +++ b/tests/ui/lifetimes/lint-macro-extended-temporary-scopes/extended-super-let-bindings.rs @@ -7,5 +7,6 @@ fn main() { // The `()` argument to the inner `format_args!` is promoted, but the lifetimes of the internal // `super let` temporaries in its expansion shorten, making this an error in Rust 1.92. println!("{:?}{}", (), { format_args!("{:?}", ()) }); - // TODO: warn + //~^ WARN temporary lifetime will be shortened in Rust 1.92 + //~| WARN this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! } diff --git a/tests/ui/lifetimes/lint-macro-extended-temporary-scopes/extended-super-let-bindings.stderr b/tests/ui/lifetimes/lint-macro-extended-temporary-scopes/extended-super-let-bindings.stderr new file mode 100644 index 000000000000..4dee6f4fc321 --- /dev/null +++ b/tests/ui/lifetimes/lint-macro-extended-temporary-scopes/extended-super-let-bindings.stderr @@ -0,0 +1,15 @@ +warning: temporary lifetime will be shortened in Rust 1.92 + --> $DIR/extended-super-let-bindings.rs:9:30 + | +LL | println!("{:?}{}", (), { format_args!("{:?}", ()) }); + | ^^^^^^^^^^^^^^^^^^^^^^^^ - ...which will be dropped at the end of this block in Rust 1.92 + | | + | this expression creates a temporary value... + | + = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! + = note: for more information, see + = note: consider using a `let` binding to create a longer lived value + = note: `#[warn(macro_extended_temporary_scopes)]` (part of `#[warn(future_incompatible)]`) on by default + +warning: 1 warning emitted +