warn on shortening super let binding lifetimes

Author: dianne

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<Scope>,
+    var_parent: (Option<Scope>, ScopeCompatibility),
 
     /// Region parent of expressions, etc.
     parent: Option<Scope>,
@@ -56,14 +56,18 @@ 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
             //
             // extern fn isalnum(c: c_int) -> c_int
         }
         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,15 +214,15 @@ 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 {
         // We introduce a new scope to contain bindings and temporaries from `if let` guards, to
         // 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(),
         };
 

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;

compiler/rustc_middle/src/middle/region.rs

@@ -222,6 +222,10 @@ pub struct ScopeTree {
     /// variable is declared.
     var_map: FxIndexMap<hir::ItemLocalId, Scope>,
 
+    /// Maps from bindings to their future scopes after #145838 for the
+    /// `macro_extended_temporary_scopes` lint.
+    var_compatibility_map: FxIndexMap<hir::ItemLocalId, Scope>,
+
     /// 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<Scope> {
-        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<Scope>, 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

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);
         }
     }

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(

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);
     }

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

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 <https://doc.rust-lang.org/rustc/lints/listing/warn-by-default.html#macro-extended-temporary-scopes>
+   = 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
+