only extend temporaries when necessary

Author: dianne

compiler/rustc_hir_analysis/src/check/region.rs

@@ -14,7 +14,6 @@ use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::{self, Visitor};
 use rustc_hir::{Arm, Block, Expr, LetStmt, Pat, PatKind, Stmt};
 use rustc_index::Idx;
-use rustc_middle::bug;
 use rustc_middle::middle::region::*;
 use rustc_middle::thir::TempLifetime;
 use rustc_middle::ty::TyCtxt;
@@ -30,8 +29,9 @@ struct Context {
     /// Region parent of expressions, etc.
     parent: Option<Scope>,
 
-    /// Scope of lifetime-extended temporaries.
-    extended_parent: ExtendedScope,
+    /// Scope of lifetime-extended temporaries. If `None`, extendable expressions have their usual
+    /// temporary scopes. This helps minimize insertions to the [`ScopeTree`]'s `rvalue_scopes` map.
+    extended_parent: Option<ExtendedScope>,
 }
 
 /// Determines the scopes of subexpressions' temporaries.
@@ -56,9 +56,6 @@ enum ExtendedScope {
     /// Extendable temporaries will be dropped in the temporary scope enclosing the given scope.
     /// This is a separate variant to minimize calls to [`ScopeTree::default_temporary_scope`].
     ThroughExpression(Scope),
-    /// The outermost extended scope is set when visiting a body. To make sure we don't forget to do
-    /// so, this dummy variant is used when creating the [`ScopeResolutionVisitor`].
-    Unset,
 }
 
 impl ExtendedScope {
@@ -72,7 +69,6 @@ impl ExtendedScope {
                     scope_tree.default_temporary_scope(non_extending_parent);
                 TempLifetime { temp_lifetime: Some(temp_scope), backwards_incompatible }
             }
-            ExtendedScope::Unset => bug!("extended parent scope not set"),
         }
     }
 }
@@ -204,6 +200,14 @@ fn resolve_block<'tcx>(
                     .scope_tree
                     .backwards_incompatible_scope
                     .insert(local_id, Scope { local_id, data: ScopeData::Node });
+
+                // To avoid false positives in `tail_expr_drop_order`, make sure extendable
+                // temporaries are extended past the block tail even if that doesn't change their
+                // scopes in the current edition.
+                if visitor.cx.extended_parent.is_none() {
+                    visitor.cx.extended_parent =
+                        Some(ExtendedScope::ThroughExpression(visitor.cx.parent.unwrap()));
+                }
             }
             resolve_expr(visitor, tail_expr, NodeInfo { drop_temps, extending: true });
         }
@@ -320,10 +324,9 @@ fn resolve_expr<'tcx>(
     //        | E& as ...
     match expr.kind {
         hir::ExprKind::AddrOf(_, _, subexpr) => {
-            // Record an extended lifetime if we haven't already done so. This makes sure we don't
-            // overwrite the scope of e.g. `temp()` in `&*&temp()` when visiting the inner `&`.
-            if !visitor.scope_tree.has_extended_scope(subexpr.hir_id.local_id) {
-                let lifetime = visitor.cx.extended_parent.to_scope(&visitor.scope_tree);
+            // Record an extended lifetime for the operand if needed.
+            if let Some(extended_scope) = visitor.cx.extended_parent {
+                let lifetime = extended_scope.to_scope(&visitor.scope_tree);
                 record_subexpr_rvalue_scopes(&mut visitor.scope_tree, subexpr, lifetime);
             }
             resolve_expr(visitor, subexpr, NodeInfo { drop_temps: false, extending: true });
@@ -589,8 +592,13 @@ fn resolve_local<'tcx>(
         // FIXME(super_let): This ignores backward-incompatible drop hints. Implementing BIDs for
         // `super let` bindings could improve `tail_expr_drop_order` with regard to `pin!`, etc.
 
-        visitor.cx.var_parent =
-            visitor.cx.extended_parent.to_scope(&visitor.scope_tree).temp_lifetime;
+        visitor.cx.var_parent = match visitor.cx.extended_parent {
+            Some(extended_parent) => extended_parent.to_scope(&visitor.scope_tree).temp_lifetime,
+            None => visitor
+                .cx
+                .var_parent
+                .map(|block| visitor.scope_tree.default_temporary_scope(block).0),
+        };
     }
 
     if let Some(expr) = init {
@@ -610,7 +618,8 @@ fn resolve_local<'tcx>(
             // When visiting the initializer, extend borrows and `super let`s accessible through
             // extending subexpressions to live in the current variable scope (or in the case of
             // statics and consts, for the whole program).
-            visitor.cx.extended_parent = ExtendedScope::ThroughDeclaration(visitor.cx.var_parent);
+            visitor.cx.extended_parent =
+                Some(ExtendedScope::ThroughDeclaration(visitor.cx.var_parent));
         }
 
         // Make sure we visit the initializer first.
@@ -759,14 +768,20 @@ impl<'tcx> ScopeResolutionVisitor<'tcx> {
         // account for the destruction scope representing the scope of
         // the destructors that run immediately after it completes.
         if node_info.drop_temps {
+            // If this scope corresponds to an extending subexpression, we can extend certain
+            // temporaries' scopes through it.
+            if node_info.extending && self.cx.extended_parent.is_none() {
+                self.cx.extended_parent = Some(ExtendedScope::ThroughExpression(
+                    self.cx.parent.expect("extending subexpressions should have parent scopes"),
+                ));
+            }
             self.enter_scope(Scope { local_id: id, data: ScopeData::Destruction });
         }
         self.enter_scope(Scope { local_id: id, data: ScopeData::Node });
         // If this scope corresponds to a non-extending subexpression, limit the scopes of
         // temporaries to the enclosing temporary scope.
         if !node_info.extending {
-            self.cx.extended_parent =
-                ExtendedScope::ThroughExpression(Scope { local_id: id, data: ScopeData::Node });
+            self.cx.extended_parent = None;
         }
     }
 
@@ -881,7 +896,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, extended_parent: ExtendedScope::Unset },
+            cx: Context { parent: None, var_parent: None, extended_parent: None },
         };
 
         visitor.scope_tree.root_body = Some(body.value.hir_id);

compiler/rustc_middle/src/middle/region.rs

@@ -255,13 +255,8 @@ impl ScopeTree {
         if let Some(lifetime) = lifetime.temp_lifetime {
             assert!(var != lifetime.local_id);
         }
-        let result = self.rvalue_scopes.try_insert(var, lifetime);
-        assert!(result.is_ok());
-    }
-
-    /// Check for an association between a sub-expression and an extended lifetime
-    pub fn has_extended_scope(&self, var: hir::ItemLocalId) -> bool {
-        self.rvalue_scopes.contains_key(&var)
+        let old_lifetime = self.rvalue_scopes.insert(var, lifetime);
+        assert!(old_lifetime.is_none_or(|old| old == lifetime));
     }
 
     /// Returns the narrowest scope that encloses `id`, if any.

compiler/rustc_middle/src/thir.rs

@@ -265,7 +265,7 @@ pub struct Expr<'tcx> {
 }
 
 /// Temporary lifetime information for THIR expressions
-#[derive(Clone, Copy, Debug, HashStable)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq, HashStable)]
 pub struct TempLifetime {
     /// Lifetime for temporaries as expected.
     /// This should be `None` in a constant context.