Remove terminating_scopes hash set.

compiler/rustc_hir_analysis/src/check/region.rs

@@ -8,7 +8,6 @@
 
 use std::mem;
 
-use rustc_data_structures::fx::FxHashSet;
 use rustc_hir as hir;
 use rustc_hir::def_id::DefId;
 use rustc_hir::intravisit::{self, Visitor};
@@ -45,28 +44,6 @@ struct ScopeResolutionVisitor<'tcx> {
     scope_tree: ScopeTree,
 
     cx: Context,
-
-    /// `terminating_scopes` is a set containing the ids of each
-    /// statement, or conditional/repeating expression. These scopes
-    /// are calling "terminating scopes" because, when attempting to
-    /// find the scope of a temporary, by default we search up the
-    /// enclosing scopes until we encounter the terminating scope. A
-    /// conditional/repeating expression is one which is not
-    /// guaranteed to execute exactly once upon entering the parent
-    /// scope. This could be because the expression only executes
-    /// conditionally, such as the expression `b` in `a && b`, or
-    /// because the expression may execute many times, such as a loop
-    /// body. The reason that we distinguish such expressions is that,
-    /// upon exiting the parent scope, we cannot statically know how
-    /// many times the expression executed, and thus if the expression
-    /// creates temporaries we cannot know statically how many such
-    /// temporaries we would have to cleanup. Therefore, we ensure that
-    /// the temporaries never outlast the conditional/repeating
-    /// expression, preventing the need for dynamic checks and/or
-    /// arbitrary amounts of stack space. Terminating scopes end
-    /// up being contained in a DestructionScope that contains the
-    /// destructor's execution.
-    terminating_scopes: FxHashSet<hir::ItemLocalId>,
 }
 
 /// Records the lifetime of a local variable as `cx.var_parent`
@@ -81,7 +58,11 @@ fn record_var_lifetime(visitor: &mut ScopeResolutionVisitor<'_>, var_id: hir::It
     }
 }
 
-fn resolve_block<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, blk: &'tcx hir::Block<'tcx>) {
+fn resolve_block<'tcx>(
+    visitor: &mut ScopeResolutionVisitor<'tcx>,
+    blk: &'tcx hir::Block<'tcx>,
+    terminating: bool,
+) {
     debug!("resolve_block(blk.hir_id={:?})", blk.hir_id);
 
     let prev_cx = visitor.cx;
@@ -111,7 +92,7 @@ fn resolve_block<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, blk: &'tcx hi
     // `other_argument()` has run and also the call to `quux(..)`
     // itself has returned.
 
-    visitor.enter_node_scope_with_dtor(blk.hir_id.local_id);
+    visitor.enter_node_scope_with_dtor(blk.hir_id.local_id, terminating);
     visitor.cx.var_parent = visitor.cx.parent;
 
     {
@@ -140,8 +121,7 @@ fn resolve_block<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, blk: &'tcx hi
                     // the sequence of visits agree with the order in the default
                     // `hir::intravisit` visitor.
                     mem::swap(&mut prev_cx, &mut visitor.cx);
-                    visitor.terminating_scopes.insert(els.hir_id.local_id);
+                    resolve_block(visitor, els, true);
-                    visitor.visit_block(els);
                     // From now on, we continue normally.
                     visitor.cx = prev_cx;
                 }
@@ -169,12 +149,12 @@ fn resolve_block<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, blk: &'tcx hi
         if let Some(tail_expr) = blk.expr {
             let local_id = tail_expr.hir_id.local_id;
             let edition = blk.span.edition();
-            if edition.at_least_rust_2024() {
+            let terminating = edition.at_least_rust_2024();
-                visitor.terminating_scopes.insert(local_id);
+            if !terminating
-            } else if !visitor
+                && !visitor
-                .tcx
+                    .tcx
-                .lints_that_dont_need_to_run(())
+                    .lints_that_dont_need_to_run(())
-                .contains(&lint::LintId::of(lint::builtin::TAIL_EXPR_DROP_ORDER))
+                    .contains(&lint::LintId::of(lint::builtin::TAIL_EXPR_DROP_ORDER))
             {
                 // If this temporary scope will be changing once the codebase adopts Rust 2024,
                 // and we are linting about possible semantic changes that would result,
@@ -185,7 +165,7 @@ fn resolve_block<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, blk: &'tcx hi
                     .backwards_incompatible_scope
                     .insert(local_id, Scope { local_id, data: ScopeData::Node });
             }
-            visitor.visit_expr(tail_expr);
+            resolve_expr(visitor, tail_expr, terminating);
         }
     }
 
@@ -203,18 +183,14 @@ fn resolve_arm<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, arm: &'tcx hir:
 
     let prev_cx = visitor.cx;
 
-    visitor.terminating_scopes.insert(arm.hir_id.local_id);
+    visitor.enter_node_scope_with_dtor(arm.hir_id.local_id, true);
-
-    visitor.enter_node_scope_with_dtor(arm.hir_id.local_id);
     visitor.cx.var_parent = visitor.cx.parent;
 
-    if let Some(expr) = arm.guard
+    resolve_pat(visitor, arm.pat);
-        && !has_let_expr(expr)
+    if let Some(guard) = arm.guard {
-    {
+        resolve_expr(visitor, guard, !has_let_expr(guard));
-        visitor.terminating_scopes.insert(expr.hir_id.local_id);
     }
-
+    resolve_expr(visitor, arm.body, false);
-    intravisit::walk_arm(visitor, arm);
 
     visitor.cx = prev_cx;
 }
@@ -243,126 +219,24 @@ fn resolve_stmt<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, stmt: &'tcx hi
     // associated destruction scope that represents the scope of the
     // statement plus its destructors, and thus the scope for which
     // regions referenced by the destructors need to survive.
-    visitor.terminating_scopes.insert(stmt_id);
 
     let prev_parent = visitor.cx.parent;
-    visitor.enter_node_scope_with_dtor(stmt_id);
+    visitor.enter_node_scope_with_dtor(stmt_id, true);
 
     intravisit::walk_stmt(visitor, stmt);
 
     visitor.cx.parent = prev_parent;
 }
 
-fn resolve_expr<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, expr: &'tcx hir::Expr<'tcx>) {
+fn resolve_expr<'tcx>(
+    visitor: &mut ScopeResolutionVisitor<'tcx>,
+    expr: &'tcx hir::Expr<'tcx>,
+    terminating: bool,
+) {
     debug!("resolve_expr - pre-increment {} expr = {:?}", visitor.expr_and_pat_count, expr);
 
     let prev_cx = visitor.cx;
-    visitor.enter_node_scope_with_dtor(expr.hir_id.local_id);
+    visitor.enter_node_scope_with_dtor(expr.hir_id.local_id, terminating);
-
-    {
-        let terminating_scopes = &mut visitor.terminating_scopes;
-        let mut terminating = |id: hir::ItemLocalId| {
-            terminating_scopes.insert(id);
-        };
-        match expr.kind {
-            // Conditional or repeating scopes are always terminating
-            // scopes, meaning that temporaries cannot outlive them.
-            // This ensures fixed size stacks.
-            hir::ExprKind::Binary(
-                source_map::Spanned { node: hir::BinOpKind::And | hir::BinOpKind::Or, .. },
-                l,
-                r,
-            ) => {
-                // expr is a short circuiting operator (|| or &&). As its
-                // functionality can't be overridden by traits, it always
-                // processes bool sub-expressions. bools are Copy and thus we
-                // can drop any temporaries in evaluation (read) order
-                // (with the exception of potentially failing let expressions).
-                // We achieve this by enclosing the operands in a terminating
-                // scope, both the LHS and the RHS.
-
-                // We optimize this a little in the presence of chains.
-                // Chains like a && b && c get lowered to AND(AND(a, b), c).
-                // In here, b and c are RHS, while a is the only LHS operand in
-                // that chain. This holds true for longer chains as well: the
-                // leading operand is always the only LHS operand that is not a
-                // binop itself. Putting a binop like AND(a, b) into a
-                // terminating scope is not useful, thus we only put the LHS
-                // into a terminating scope if it is not a binop.
-
-                let terminate_lhs = match l.kind {
-                    // let expressions can create temporaries that live on
-                    hir::ExprKind::Let(_) => false,
-                    // binops already drop their temporaries, so there is no
-                    // need to put them into a terminating scope.
-                    // This is purely an optimization to reduce the number of
-                    // terminating scopes.
-                    hir::ExprKind::Binary(
-                        source_map::Spanned {
-                            node: hir::BinOpKind::And | hir::BinOpKind::Or, ..
-                        },
-                        ..,
-                    ) => false,
-                    // otherwise: mark it as terminating
-                    _ => true,
-                };
-                if terminate_lhs {
-                    terminating(l.hir_id.local_id);
-                }
-
-                // `Let` expressions (in a let-chain) shouldn't be terminating, as their temporaries
-                // should live beyond the immediate expression
-                if !matches!(r.kind, hir::ExprKind::Let(_)) {
-                    terminating(r.hir_id.local_id);
-                }
-            }
-            hir::ExprKind::If(_, then, Some(otherwise)) => {
-                terminating(then.hir_id.local_id);
-                terminating(otherwise.hir_id.local_id);
-            }
-
-            hir::ExprKind::If(_, then, None) => {
-                terminating(then.hir_id.local_id);
-            }
-
-            hir::ExprKind::Loop(body, _, _, _) => {
-                terminating(body.hir_id.local_id);
-            }
-
-            hir::ExprKind::DropTemps(expr) => {
-                // `DropTemps(expr)` does not denote a conditional scope.
-                // Rather, we want to achieve the same behavior as `{ let _t = expr; _t }`.
-                terminating(expr.hir_id.local_id);
-            }
-
-            hir::ExprKind::AssignOp(..)
-            | hir::ExprKind::Index(..)
-            | hir::ExprKind::Unary(..)
-            | hir::ExprKind::Call(..)
-            | hir::ExprKind::MethodCall(..) => {
-                // FIXME(https://github.com/rust-lang/rfcs/issues/811) Nested method calls
-                //
-                // The lifetimes for a call or method call look as follows:
-                //
-                // call.id
-                // - arg0.id
-                // - ...
-                // - argN.id
-                // - call.callee_id
-                //
-                // The idea is that call.callee_id represents *the time when
-                // the invoked function is actually running* and call.id
-                // represents *the time to prepare the arguments and make the
-                // call*. See the section "Borrows in Calls" borrowck/README.md
-                // for an extended explanation of why this distinction is
-                // important.
-                //
-                // record_superlifetime(new_cx, expr.callee_id);
-            }
-
-            _ => {}
-        }
-    }
 
     let prev_pessimistic = visitor.pessimistic_yield;
 
@@ -417,6 +291,53 @@ fn resolve_expr<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, expr: &'tcx hi
     // properly, we can't miss any types.
 
     match expr.kind {
+        // Conditional or repeating scopes are always terminating
+        // scopes, meaning that temporaries cannot outlive them.
+        // This ensures fixed size stacks.
+        hir::ExprKind::Binary(
+            source_map::Spanned { node: hir::BinOpKind::And | hir::BinOpKind::Or, .. },
+            left,
+            right,
+        ) => {
+            // expr is a short circuiting operator (|| or &&). As its
+            // functionality can't be overridden by traits, it always
+            // processes bool sub-expressions. bools are Copy and thus we
+            // can drop any temporaries in evaluation (read) order
+            // (with the exception of potentially failing let expressions).
+            // We achieve this by enclosing the operands in a terminating
+            // scope, both the LHS and the RHS.
+
+            // We optimize this a little in the presence of chains.
+            // Chains like a && b && c get lowered to AND(AND(a, b), c).
+            // In here, b and c are RHS, while a is the only LHS operand in
+            // that chain. This holds true for longer chains as well: the
+            // leading operand is always the only LHS operand that is not a
+            // binop itself. Putting a binop like AND(a, b) into a
+            // terminating scope is not useful, thus we only put the LHS
+            // into a terminating scope if it is not a binop.
+
+            let terminate_lhs = match left.kind {
+                // let expressions can create temporaries that live on
+                hir::ExprKind::Let(_) => false,
+                // binops already drop their temporaries, so there is no
+                // need to put them into a terminating scope.
+                // This is purely an optimization to reduce the number of
+                // terminating scopes.
+                hir::ExprKind::Binary(
+                    source_map::Spanned { node: hir::BinOpKind::And | hir::BinOpKind::Or, .. },
+                    ..,
+                ) => false,
+                // otherwise: mark it as terminating
+                _ => true,
+            };
+
+            // `Let` expressions (in a let-chain) shouldn't be terminating, as their temporaries
+            // should live beyond the immediate expression
+            let terminate_rhs = !matches!(right.kind, hir::ExprKind::Let(_));
+
+            resolve_expr(visitor, left, terminate_lhs);
+            resolve_expr(visitor, right, terminate_rhs);
+        }
         // Manually recurse over closures, because they are nested bodies
         // that share the parent environment. We handle const blocks in
         // `visit_inline_const`.
@@ -485,9 +406,9 @@ fn resolve_expr<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, expr: &'tcx hi
             visitor.enter_scope(Scope { local_id: then.hir_id.local_id, data });
             visitor.cx.var_parent = visitor.cx.parent;
             visitor.visit_expr(cond);
-            visitor.visit_expr(then);
+            resolve_expr(visitor, then, true);
             visitor.cx = expr_cx;
-            visitor.visit_expr(otherwise);
+            resolve_expr(visitor, otherwise, true);
         }
 
         hir::ExprKind::If(cond, then, None) => {
@@ -500,10 +421,20 @@ fn resolve_expr<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, expr: &'tcx hi
             visitor.enter_scope(Scope { local_id: then.hir_id.local_id, data });
             visitor.cx.var_parent = visitor.cx.parent;
             visitor.visit_expr(cond);
-            visitor.visit_expr(then);
+            resolve_expr(visitor, then, true);
             visitor.cx = expr_cx;
         }
 
+        hir::ExprKind::Loop(body, _, _, _) => {
+            resolve_block(visitor, body, true);
+        }
+
+        hir::ExprKind::DropTemps(expr) => {
+            // `DropTemps(expr)` does not denote a conditional scope.
+            // Rather, we want to achieve the same behavior as `{ let _t = expr; _t }`.
+            resolve_expr(visitor, expr, true);
+        }
+
         _ => intravisit::walk_expr(visitor, expr),
     }
 
@@ -786,12 +717,12 @@ impl<'tcx> ScopeResolutionVisitor<'tcx> {
         self.cx.parent = Some(child_scope);
     }
 
-    fn enter_node_scope_with_dtor(&mut self, id: hir::ItemLocalId) {
+    fn enter_node_scope_with_dtor(&mut self, id: hir::ItemLocalId, terminating: bool) {
         // If node was previously marked as a terminating scope during the
         // recursive visit of its parent node in the HIR, then we need to
         // account for the destruction scope representing the scope of
         // the destructors that run immediately after it completes.
-        if self.terminating_scopes.contains(&id) {
+        if terminating {
             self.enter_scope(Scope { local_id: id, data: ScopeData::Destruction });
         }
         self.enter_scope(Scope { local_id: id, data: ScopeData::Node });
@@ -803,13 +734,11 @@ impl<'tcx> ScopeResolutionVisitor<'tcx> {
         // visited the body.
         let outer_ec = mem::replace(&mut self.expr_and_pat_count, 0);
         let outer_cx = self.cx;
-        let outer_ts = mem::take(&mut self.terminating_scopes);
         // The 'pessimistic yield' flag is set to true when we are
         // processing a `+=` statement and have to make pessimistic
         // control flow assumptions. This doesn't apply to nested
         // bodies within the `+=` statements. See #69307.
         let outer_pessimistic_yield = mem::replace(&mut self.pessimistic_yield, false);
-        self.terminating_scopes.insert(hir_id.local_id);
 
         self.enter_scope(Scope { local_id: hir_id.local_id, data: ScopeData::CallSite });
         self.enter_scope(Scope { local_id: hir_id.local_id, data: ScopeData::Arguments });
@@ -819,14 +748,13 @@ impl<'tcx> ScopeResolutionVisitor<'tcx> {
         // Restore context we had at the start.
         self.expr_and_pat_count = outer_ec;
         self.cx = outer_cx;
-        self.terminating_scopes = outer_ts;
         self.pessimistic_yield = outer_pessimistic_yield;
     }
 }
 
 impl<'tcx> Visitor<'tcx> for ScopeResolutionVisitor<'tcx> {
     fn visit_block(&mut self, b: &'tcx Block<'tcx>) {
-        resolve_block(self, b);
+        resolve_block(self, b, false);
     }
 
     fn visit_body(&mut self, body: &hir::Body<'tcx>) {
@@ -850,7 +778,7 @@ impl<'tcx> Visitor<'tcx> for ScopeResolutionVisitor<'tcx> {
                 }
 
                 // The body of the every fn is a root scope.
-                this.visit_expr(body.value)
+                resolve_expr(this, body.value, true);
             } else {
                 // Only functions have an outer terminating (drop) scope, while
                 // temporaries in constant initializers may be 'static, but only
@@ -871,6 +799,10 @@ impl<'tcx> Visitor<'tcx> for ScopeResolutionVisitor<'tcx> {
                 // (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.enter_scope(Scope {
+                    local_id: body.value.hir_id.local_id,
+                    data: ScopeData::Destruction,
+                });
                 resolve_local(this, None, Some(body.value));
             }
         })
@@ -886,7 +818,7 @@ impl<'tcx> Visitor<'tcx> for ScopeResolutionVisitor<'tcx> {
         resolve_stmt(self, s);
     }
     fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
-        resolve_expr(self, ex);
+        resolve_expr(self, ex, false);
     }
     fn visit_local(&mut self, l: &'tcx LetStmt<'tcx>) {
         resolve_local(self, Some(l.pat), l.init)
@@ -916,7 +848,6 @@ pub(crate) fn region_scope_tree(tcx: TyCtxt<'_>, def_id: DefId) -> &ScopeTree {
             scope_tree: ScopeTree::default(),
             expr_and_pat_count: 0,
             cx: Context { parent: None, var_parent: None },
-            terminating_scopes: Default::default(),
             pessimistic_yield: false,
             fixup_scopes: vec![],
         };