user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Rollup merge of #143494 - cjgillot:no-yield-in-scope, r=compiler-errors

Browse source 
Remove yields_in_scope from the scope tree.

I believe this has not been in use since we removed the HIR-based generator interior type computation.
This commit is contained in:
Matthias Krüger 2025-07-05 22:34:43 +02:00 • committed by GitHub
commit c3c4fd7c9c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 37 additions and 263 deletions
Download patch file Download diff file Expand all files Collapse all files

205
compiler/rustc_hir_analysis/src/check/region.rs
View file

@ -15,7 +15,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::ty::TyCtxt;
use rustc_session::lint;
@ -34,14 +33,6 @@ struct Context {
struct ScopeResolutionVisitor<'tcx> {
tcx: TyCtxt<'tcx>,
// The number of expressions and patterns visited in the current body.
expr_and_pat_count: usize,
// When this is `true`, we record the `Scopes` we encounter
// when processing a Yield expression. This allows us to fix
// up their indices.
pessimistic_yield: bool,
// Stores scopes when `pessimistic_yield` is `true`.
fixup_scopes: Vec<Scope>,
// The generated scope tree.
scope_tree: ScopeTree,
@ -199,19 +190,14 @@ fn resolve_arm<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, arm: &'tcx hir:
visitor.cx = prev_cx;
}
#[tracing::instrument(level = "debug", skip(visitor))]
fn resolve_pat<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, pat: &'tcx hir::Pat<'tcx>) {
// If this is a binding then record the lifetime of that binding.
if let PatKind::Binding(..) = pat.kind {
record_var_lifetime(visitor, pat.hir_id.local_id);
}
debug!("resolve_pat - pre-increment {} pat = {:?}", visitor.expr_and_pat_count, pat);
intravisit::walk_pat(visitor, pat);
visitor.expr_and_pat_count += 1;
debug!("resolve_pat - post-increment {} pat = {:?}", visitor.expr_and_pat_count, pat);
}
fn resolve_stmt<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, stmt: &'tcx hir::Stmt<'tcx>) {
@ -243,68 +229,15 @@ fn resolve_stmt<'tcx>(visitor: &mut ScopeResolutionVisitor<'tcx>, stmt: &'tcx hi
}
}
#[tracing::instrument(level = "debug", skip(visitor))]
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, terminating);
let prev_pessimistic = visitor.pessimistic_yield;
// Ordinarily, we can rely on the visit order of HIR intravisit
// to correspond to the actual execution order of statements.
// However, there's a weird corner case with compound assignment
// operators (e.g. `a += b`). The evaluation order depends on whether
// or not the operator is overloaded (e.g. whether or not a trait
// like AddAssign is implemented).
// For primitive types (which, despite having a trait impl, don't actually
// end up calling it), the evaluation order is right-to-left. For example,
// the following code snippet:
//
// let y = &mut 0;
// *{println!("LHS!"); y} += {println!("RHS!"); 1};
//
// will print:
//
// RHS!
// LHS!
//
// However, if the operator is used on a non-primitive type,
// the evaluation order will be left-to-right, since the operator
// actually get desugared to a method call. For example, this
// nearly identical code snippet:
//
// let y = &mut String::new();
// *{println!("LHS String"); y} += {println!("RHS String"); "hi"};
//
// will print:
// LHS String
// RHS String
//
// To determine the actual execution order, we need to perform
// trait resolution. Unfortunately, we need to be able to compute
// yield_in_scope before type checking is even done, as it gets
// used by AST borrowcheck.
//
// Fortunately, we don't need to know the actual execution order.
// It suffices to know the 'worst case' order with respect to yields.
// Specifically, we need to know the highest 'expr_and_pat_count'
// that we could assign to the yield expression. To do this,
// we pick the greater of the two values from the left-hand
// and right-hand expressions. This makes us overly conservative
// about what types could possibly live across yield points,
// but we will never fail to detect that a type does actually
// live across a yield point. The latter part is critical -
// we're already overly conservative about what types will live
// across yield points, as the generated MIR will determine
// when things are actually live. However, for typecheck to work
// properly, we can't miss any types.
match expr.kind {
// Conditional or repeating scopes are always terminating
// scopes, meaning that temporaries cannot outlive them.
@ -360,55 +293,42 @@ fn resolve_expr<'tcx>(
let body = visitor.tcx.hir_body(body);
visitor.visit_body(body);
}
// Ordinarily, we can rely on the visit order of HIR intravisit
// to correspond to the actual execution order of statements.
// However, there's a weird corner case with compound assignment
// operators (e.g. `a += b`). The evaluation order depends on whether
// or not the operator is overloaded (e.g. whether or not a trait
// like AddAssign is implemented).
//
// For primitive types (which, despite having a trait impl, don't actually
// end up calling it), the evaluation order is right-to-left. For example,
// the following code snippet:
//
// let y = &mut 0;
// *{println!("LHS!"); y} += {println!("RHS!"); 1};
//
// will print:
//
// RHS!
// LHS!
//
// However, if the operator is used on a non-primitive type,
// the evaluation order will be left-to-right, since the operator
// actually get desugared to a method call. For example, this
// nearly identical code snippet:
//
// let y = &mut String::new();
// *{println!("LHS String"); y} += {println!("RHS String"); "hi"};
//
// will print:
// LHS String
// RHS String
//
// To determine the actual execution order, we need to perform
// trait resolution. Fortunately, we don't need to know the actual execution order.
hir::ExprKind::AssignOp(_, left_expr, right_expr) => {
debug!(
"resolve_expr - enabling pessimistic_yield, was previously {}",
prev_pessimistic
);
let start_point = visitor.fixup_scopes.len();
visitor.pessimistic_yield = true;
// If the actual execution order turns out to be right-to-left,
// then we're fine. However, if the actual execution order is left-to-right,
// then we'll assign too low a count to any `yield` expressions
// we encounter in 'right_expression' - they should really occur after all of the
// expressions in 'left_expression'.
visitor.visit_expr(right_expr);
visitor.pessimistic_yield = prev_pessimistic;
debug!("resolve_expr - restoring pessimistic_yield to {}", prev_pessimistic);
visitor.visit_expr(left_expr);
debug!("resolve_expr - fixing up counts to {}", visitor.expr_and_pat_count);
// Remove and process any scopes pushed by the visitor
let target_scopes = visitor.fixup_scopes.drain(start_point..);
for scope in target_scopes {
let yield_data =
visitor.scope_tree.yield_in_scope.get_mut(&scope).unwrap().last_mut().unwrap();
let count = yield_data.expr_and_pat_count;
let span = yield_data.span;
// expr_and_pat_count never decreases. Since we recorded counts in yield_in_scope
// before walking the left-hand side, it should be impossible for the recorded
// count to be greater than the left-hand side count.
if count > visitor.expr_and_pat_count {
bug!(
"Encountered greater count {} at span {:?} - expected no greater than {}",
count,
span,
visitor.expr_and_pat_count
);
}
let new_count = visitor.expr_and_pat_count;
debug!(
"resolve_expr - increasing count for scope {:?} from {} to {} at span {:?}",
scope, count, new_count, span
);
yield_data.expr_and_pat_count = new_count;
}
}
hir::ExprKind::If(cond, then, Some(otherwise)) => {
@ -453,43 +373,6 @@ fn resolve_expr<'tcx>(
_ => intravisit::walk_expr(visitor, expr),
}
visitor.expr_and_pat_count += 1;
debug!("resolve_expr post-increment {}, expr = {:?}", visitor.expr_and_pat_count, expr);
if let hir::ExprKind::Yield(_, source) = &expr.kind {
// Mark this expr's scope and all parent scopes as containing `yield`.
let mut scope = Scope { local_id: expr.hir_id.local_id, data: ScopeData::Node };
loop {
let data = YieldData {
span: expr.span,
expr_and_pat_count: visitor.expr_and_pat_count,
source: *source,
};
match visitor.scope_tree.yield_in_scope.get_mut(&scope) {
Some(yields) => yields.push(data),
None => {
visitor.scope_tree.yield_in_scope.insert(scope, vec![data]);
}
}
if visitor.pessimistic_yield {
debug!("resolve_expr in pessimistic_yield - marking scope {:?} for fixup", scope);
visitor.fixup_scopes.push(scope);
}
// Keep traversing up while we can.
match visitor.scope_tree.parent_map.get(&scope) {
// Don't cross from closure bodies to their parent.
Some(&superscope) => match superscope.data {
ScopeData::CallSite => break,
_ => scope = superscope,
},
None => break,
}
}
}
visitor.cx = prev_cx;
}
@ -612,8 +495,8 @@ fn resolve_local<'tcx>(
}
}
// Make sure we visit the initializer first, so expr_and_pat_count remains correct.
// The correct order, as shared between coroutine_interior, drop_ranges and intravisitor,
// Make sure we visit the initializer first.
// The correct order, as shared between drop_ranges and intravisitor,
// is to walk initializer, followed by pattern bindings, finally followed by the `else` block.
if let Some(expr) = init {
visitor.visit_expr(expr);
@ -798,16 +681,7 @@ impl<'tcx> ScopeResolutionVisitor<'tcx> {
}
fn enter_body(&mut self, hir_id: hir::HirId, f: impl FnOnce(&mut Self)) {
// Save all state that is specific to the outer function
// body. These will be restored once down below, once we've
// visited the body.
let outer_ec = mem::replace(&mut self.expr_and_pat_count, 0);
let outer_cx = self.cx;
// 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.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 });
@ -815,9 +689,7 @@ impl<'tcx> ScopeResolutionVisitor<'tcx> {
f(self);
// Restore context we had at the start.
self.expr_and_pat_count = outer_ec;
self.cx = outer_cx;
self.pessimistic_yield = outer_pessimistic_yield;
}
}
@ -919,10 +791,7 @@ pub(crate) fn region_scope_tree(tcx: TyCtxt<'_>, def_id: DefId) -> &ScopeTree {
let mut visitor = ScopeResolutionVisitor {
tcx,
scope_tree: ScopeTree::default(),
expr_and_pat_count: 0,
cx: Context { parent: None, var_parent: None },
pessimistic_yield: false,
fixup_scopes: vec![],
extended_super_lets: Default::default(),
};

92
compiler/rustc_middle/src/middle/region.rs
View file

@ -7,7 +7,6 @@
//! [rustc dev guide]: https://rustc-dev-guide.rust-lang.org/borrow_check.html
use std::fmt;
use std::ops::Deref;
use rustc_data_structures::fx::FxIndexMap;
use rustc_data_structures::unord::UnordMap;
@ -228,82 +227,6 @@ pub struct ScopeTree {
/// This information is used later for linting to identify locals and
/// temporary values that will receive backwards-incompatible drop orders.
pub backwards_incompatible_scope: UnordMap<hir::ItemLocalId, Scope>,
/// If there are any `yield` nested within a scope, this map
/// stores the `Span` of the last one and its index in the
/// postorder of the Visitor traversal on the HIR.
///
/// HIR Visitor postorder indexes might seem like a peculiar
/// thing to care about. but it turns out that HIR bindings
/// and the temporary results of HIR expressions are never
/// storage-live at the end of HIR nodes with postorder indexes
/// lower than theirs, and therefore don't need to be suspended
/// at yield-points at these indexes.
///
/// For an example, suppose we have some code such as:
/// ```rust,ignore (example)
/// foo(f(), yield y, bar(g()))
/// ```
///
/// With the HIR tree (calls numbered for expository purposes)
///
/// ```text
/// Call#0(foo, [Call#1(f), Yield(y), Call#2(bar, Call#3(g))])
/// ```
///
/// Obviously, the result of `f()` was created before the yield
/// (and therefore needs to be kept valid over the yield) while
/// the result of `g()` occurs after the yield (and therefore
/// doesn't). If we want to infer that, we can look at the
/// postorder traversal:
/// ```plain,ignore
/// `foo` `f` Call#1 `y` Yield `bar` `g` Call#3 Call#2 Call#0
/// ```
///
/// In which we can easily see that `Call#1` occurs before the yield,
/// and `Call#3` after it.
///
/// To see that this method works, consider:
///
/// Let `D` be our binding/temporary and `U` be our other HIR node, with
/// `HIR-postorder(U) < HIR-postorder(D)`. Suppose, as in our example,
/// U is the yield and D is one of the calls.
/// Let's show that `D` is storage-dead at `U`.
///
/// Remember that storage-live/storage-dead refers to the state of
/// the *storage*, and does not consider moves/drop flags.
///
/// Then:
///
/// 1. From the ordering guarantee of HIR visitors (see
/// `rustc_hir::intravisit`), `D` does not dominate `U`.
///
/// 2. Therefore, `D` is *potentially* storage-dead at `U` (because
/// we might visit `U` without ever getting to `D`).
///
/// 3. However, we guarantee that at each HIR point, each
/// binding/temporary is always either always storage-live
/// or always storage-dead. This is what is being guaranteed
/// by `terminating_scopes` including all blocks where the
/// count of executions is not guaranteed.
///
/// 4. By `2.` and `3.`, `D` is *statically* storage-dead at `U`,
/// QED.
///
/// This property ought to not on (3) in an essential way -- it
/// is probably still correct even if we have "unrestricted" terminating
/// scopes. However, why use the complicated proof when a simple one
/// works?
///
/// A subtle thing: `box` expressions, such as `box (&x, yield 2, &y)`. It
/// might seem that a `box` expression creates a `Box<T>` temporary
/// when it *starts* executing, at `HIR-preorder(BOX-EXPR)`. That might
/// be true in the MIR desugaring, but it is not important in the semantics.
///
/// The reason is that semantically, until the `box` expression returns,
/// the values are still owned by their containing expressions. So
/// we'll see that `&x`.
pub yield_in_scope: UnordMap<Scope, Vec<YieldData>>,
}
/// See the `rvalue_candidates` field for more information on rvalue
@ -316,15 +239,6 @@ pub struct RvalueCandidate {
pub lifetime: Option<Scope>,
}
#[derive(Debug, Copy, Clone, HashStable)]
pub struct YieldData {
/// The `Span` of the yield.
pub span: Span,
/// The number of expressions and patterns appearing before the `yield` in the body, plus one.
pub expr_and_pat_count: usize,
pub source: hir::YieldSource,
}
impl ScopeTree {
pub fn record_scope_parent(&mut self, child: Scope, parent: Option<Scope>) {
debug!("{:?}.parent = {:?}", child, parent);
@ -380,10 +294,4 @@ impl ScopeTree {
true
}
/// Checks whether the given scope contains a `yield`. If so,
/// returns `Some(YieldData)`. If not, returns `None`.
pub fn yield_in_scope(&self, scope: Scope) -> Option<&[YieldData]> {
self.yield_in_scope.get(&scope).map(Deref::deref)
}
}

3
compiler/rustc_mir_build/src/builder/expr/as_rvalue.rs
View file

@ -171,9 +171,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
this.diverge_from(block);
block = success;
// The `Box<T>` temporary created here is not a part of the HIR,
// and therefore is not considered during coroutine auto-trait
// determination. See the comment about `box` at `yield_in_scope`.
let result = this.local_decls.push(LocalDecl::new(expr.ty, expr_span));
this.cfg
.push(block, Statement::new(source_info, StatementKind::StorageLive(result)));