Don't pass a break scope to Builder::break_for_else

This method would previously take a target scope, and then verify that it
was equal to the scope on top of the if-then scope stack.

In practice, this means that callers have to go out of their way to pass around
redundant scope information that's already on the if-then stack.

So it's easier to just retrieve the correct scope directly from the if-then
stack, and simplify the other code that was passing it around.
This commit is contained in:
Zalathar 2024-03-01 22:13:03 +11:00
parent 51f483944d
commit 570376c496
3 changed files with 19 additions and 38 deletions

View File

@ -83,7 +83,6 @@ pub(crate) fn expr_into_dest(
block, block,
cond, cond,
Some(condition_scope), // Temp scope Some(condition_scope), // Temp scope
condition_scope,
source_info, source_info,
true, // Declare `let` bindings normally true, // Declare `let` bindings normally
)); ));
@ -156,7 +155,6 @@ pub(crate) fn expr_into_dest(
block, block,
lhs, lhs,
Some(condition_scope), // Temp scope Some(condition_scope), // Temp scope
condition_scope,
source_info, source_info,
// This flag controls how inner `let` expressions are lowered, // This flag controls how inner `let` expressions are lowered,
// but either way there shouldn't be any of those in here. // but either way there shouldn't be any of those in here.

View File

@ -37,9 +37,6 @@ struct ThenElseArgs {
/// Used as the temp scope for lowering `expr`. If absent (for match guards), /// Used as the temp scope for lowering `expr`. If absent (for match guards),
/// `self.local_scope()` is used. /// `self.local_scope()` is used.
temp_scope_override: Option<region::Scope>, temp_scope_override: Option<region::Scope>,
/// Scope to pass to [`Builder::break_for_else`]. Must match the scope used
/// by the enclosing call to [`Builder::in_if_then_scope`].
break_scope: region::Scope,
variable_source_info: SourceInfo, variable_source_info: SourceInfo,
/// Forwarded to [`Builder::lower_let_expr`] when lowering [`ExprKind::Let`]. /// Forwarded to [`Builder::lower_let_expr`] when lowering [`ExprKind::Let`].
/// When false (for match guards), `let` bindings won't be declared. /// When false (for match guards), `let` bindings won't be declared.
@ -58,19 +55,13 @@ pub(crate) fn then_else_break(
block: BasicBlock, block: BasicBlock,
expr_id: ExprId, expr_id: ExprId,
temp_scope_override: Option<region::Scope>, temp_scope_override: Option<region::Scope>,
break_scope: region::Scope,
variable_source_info: SourceInfo, variable_source_info: SourceInfo,
declare_let_bindings: bool, declare_let_bindings: bool,
) -> BlockAnd<()> { ) -> BlockAnd<()> {
self.then_else_break_inner( self.then_else_break_inner(
block, block,
expr_id, expr_id,
ThenElseArgs { ThenElseArgs { temp_scope_override, variable_source_info, declare_let_bindings },
temp_scope_override,
break_scope,
variable_source_info,
declare_let_bindings,
},
) )
} }
@ -97,11 +88,7 @@ fn then_else_break_inner(
this.then_else_break_inner( this.then_else_break_inner(
block, block,
lhs, lhs,
ThenElseArgs { ThenElseArgs { declare_let_bindings: true, ..args },
break_scope: local_scope,
declare_let_bindings: true,
..args
},
) )
}); });
let rhs_success_block = unpack!(this.then_else_break_inner( let rhs_success_block = unpack!(this.then_else_break_inner(
@ -130,14 +117,10 @@ fn then_else_break_inner(
this.then_else_break_inner( this.then_else_break_inner(
block, block,
arg, arg,
ThenElseArgs { ThenElseArgs { declare_let_bindings: true, ..args },
break_scope: local_scope,
declare_let_bindings: true,
..args
},
) )
}); });
this.break_for_else(success_block, args.break_scope, args.variable_source_info); this.break_for_else(success_block, args.variable_source_info);
failure_block.unit() failure_block.unit()
} }
ExprKind::Scope { region_scope, lint_level, value } => { ExprKind::Scope { region_scope, lint_level, value } => {
@ -151,7 +134,6 @@ fn then_else_break_inner(
block, block,
expr, expr,
pat, pat,
args.break_scope,
Some(args.variable_source_info.scope), Some(args.variable_source_info.scope),
args.variable_source_info.span, args.variable_source_info.span,
args.declare_let_bindings, args.declare_let_bindings,
@ -170,7 +152,7 @@ fn then_else_break_inner(
let source_info = this.source_info(expr_span); let source_info = this.source_info(expr_span);
this.cfg.terminate(block, source_info, term); this.cfg.terminate(block, source_info, term);
this.break_for_else(else_block, args.break_scope, source_info); this.break_for_else(else_block, source_info);
then_block.unit() then_block.unit()
} }
@ -1911,7 +1893,6 @@ pub(crate) fn lower_let_expr(
mut block: BasicBlock, mut block: BasicBlock,
expr_id: ExprId, expr_id: ExprId,
pat: &Pat<'tcx>, pat: &Pat<'tcx>,
else_target: region::Scope,
source_scope: Option<SourceScope>, source_scope: Option<SourceScope>,
span: Span, span: Span,
declare_bindings: bool, declare_bindings: bool,
@ -1933,7 +1914,7 @@ pub(crate) fn lower_let_expr(
let expr_place = expr_place_builder.try_to_place(self); let expr_place = expr_place_builder.try_to_place(self);
let opt_expr_place = expr_place.as_ref().map(|place| (Some(place), expr_span)); let opt_expr_place = expr_place.as_ref().map(|place| (Some(place), expr_span));
let otherwise_post_guard_block = otherwise_candidate.pre_binding_block.unwrap(); let otherwise_post_guard_block = otherwise_candidate.pre_binding_block.unwrap();
self.break_for_else(otherwise_post_guard_block, else_target, self.source_info(expr_span)); self.break_for_else(otherwise_post_guard_block, self.source_info(expr_span));
if declare_bindings { if declare_bindings {
self.declare_bindings(source_scope, pat.span.to(span), pat, None, opt_expr_place); self.declare_bindings(source_scope, pat.span.to(span), pat, None, opt_expr_place);
@ -2110,7 +2091,6 @@ fn bind_and_guard_matched_candidate<'pat>(
block, block,
guard, guard,
None, // Use `self.local_scope()` as the temp scope None, // Use `self.local_scope()` as the temp scope
match_scope,
this.source_info(arm.span), this.source_info(arm.span),
false, // For guards, `let` bindings are declared separately false, // For guards, `let` bindings are declared separately
) )
@ -2443,7 +2423,7 @@ pub(crate) fn ast_let_else(
None, None,
true, true,
); );
this.break_for_else(failure, *let_else_scope, this.source_info(initializer_span)); this.break_for_else(failure, this.source_info(initializer_span));
matching.unit() matching.unit()
}); });
matching.and(failure) matching.and(failure)

View File

@ -683,20 +683,23 @@ pub(crate) fn break_scope(
self.cfg.start_new_block().unit() self.cfg.start_new_block().unit()
} }
pub(crate) fn break_for_else( /// Sets up the drops for breaking from `block` due to an `if` condition
&mut self, /// that turned out to be false.
block: BasicBlock, ///
target: region::Scope, /// Must be called in the context of [`Builder::in_if_then_scope`], so that
source_info: SourceInfo, /// there is an if-then scope to tell us what the target scope is.
) { pub(crate) fn break_for_else(&mut self, block: BasicBlock, source_info: SourceInfo) {
let scope_index = self.scopes.scope_index(target, source_info.span);
let if_then_scope = self let if_then_scope = self
.scopes .scopes
.if_then_scope .if_then_scope
.as_mut() .as_ref()
.unwrap_or_else(|| span_bug!(source_info.span, "no if-then scope found")); .unwrap_or_else(|| span_bug!(source_info.span, "no if-then scope found"));
assert_eq!(if_then_scope.region_scope, target, "breaking to incorrect scope"); let target = if_then_scope.region_scope;
let scope_index = self.scopes.scope_index(target, source_info.span);
// Upgrade `if_then_scope` to `&mut`.
let if_then_scope = self.scopes.if_then_scope.as_mut().expect("upgrading & to &mut");
let mut drop_idx = ROOT_NODE; let mut drop_idx = ROOT_NODE;
let drops = &mut if_then_scope.else_drops; let drops = &mut if_then_scope.else_drops;