diff --git a/compiler/rustc_hir_analysis/src/check/region.rs b/compiler/rustc_hir_analysis/src/check/region.rs index 2528adb937b31..cf66ab708bb9e 100644 --- a/compiler/rustc_hir_analysis/src/check/region.rs +++ b/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, } /// 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); - visitor.visit_block(els); + resolve_block(visitor, els, true); // 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() { - visitor.terminating_scopes.insert(local_id); - } else if !visitor - .tcx - .lints_that_dont_need_to_run(()) - .contains(&lint::LintId::of(lint::builtin::TAIL_EXPR_DROP_ORDER)) + let terminating = edition.at_least_rust_2024(); + if !terminating + && !visitor + .tcx + .lints_that_dont_need_to_run(()) + .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); + visitor.enter_node_scope_with_dtor(arm.hir_id.local_id, true); visitor.cx.var_parent = visitor.cx.parent; - if let Some(expr) = arm.guard - && !has_let_expr(expr) - { - visitor.terminating_scopes.insert(expr.hir_id.local_id); + resolve_pat(visitor, arm.pat); + if let Some(guard) = arm.guard { + resolve_expr(visitor, guard, !has_let_expr(guard)); } - - intravisit::walk_arm(visitor, arm); + resolve_expr(visitor, arm.body, false); 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); - - { - 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); - } - - _ => {} - } - } + visitor.enter_node_scope_with_dtor(expr.hir_id.local_id, terminating); 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![], };