Extract an arguments struct for Builder::then_else_break

Most of this method's arguments are usually or always forwarded as-is to
recursive invocations.

Wrapping them in a dedicated struct allows us to document each struct field,
and lets us use struct-update syntax to indicate which arguments are being
modified when making a recursive call.
This commit is contained in:
Zalathar 2024-02-27 17:43:07 +11:00
parent da02fff3b6
commit 4146136d6d
2 changed files with 72 additions and 66 deletions

View File

@ -81,10 +81,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let then_blk = unpack!(this.then_else_break( let then_blk = unpack!(this.then_else_break(
block, block,
cond, cond,
Some(condition_scope), Some(condition_scope), // Temp scope
condition_scope, condition_scope,
source_info, source_info,
true, true, // Declare `let` bindings normally
)); ));
this.expr_into_dest(destination, then_blk, then) this.expr_into_dest(destination, then_blk, then)
@ -146,9 +146,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
this.then_else_break( this.then_else_break(
block, block,
lhs, lhs,
Some(condition_scope), Some(condition_scope), // Temp scope
condition_scope, condition_scope,
source_info, source_info,
// This flag controls how inner `let` expressions are lowered,
// but either way there shouldn't be any of those in here.
true, true,
) )
}); });

View File

@ -30,21 +30,55 @@ mod util;
use std::borrow::Borrow; use std::borrow::Borrow;
use std::mem; use std::mem;
/// Arguments to [`Builder::then_else_break_inner`] that are usually forwarded
/// to recursive invocations.
#[derive(Clone, Copy)]
struct ThenElseArgs {
/// Used as the temp scope for lowering `expr`. If absent (for match guards),
/// `self.local_scope()` is used.
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,
/// Forwarded to [`Builder::lower_let_expr`] when lowering [`ExprKind::Let`].
/// When false (for match guards), `let` bindings won't be declared.
declare_let_bindings: bool,
}
impl<'a, 'tcx> Builder<'a, 'tcx> { impl<'a, 'tcx> Builder<'a, 'tcx> {
/// Lowers a condition in a way that ensures that variables bound in any let /// Lowers a condition in a way that ensures that variables bound in any let
/// expressions are definitely initialized in the if body. /// expressions are definitely initialized in the if body.
/// ///
/// If `declare_bindings` is false then variables created in `let` /// If `declare_let_bindings` is false then variables created in `let`
/// expressions will not be declared. This is for if let guards on arms with /// expressions will not be declared. This is for if let guards on arms with
/// an or pattern, where the guard is lowered multiple times. /// an or pattern, where the guard is lowered multiple times.
pub(crate) fn then_else_break( pub(crate) fn then_else_break(
&mut self, &mut self,
mut 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, break_scope: region::Scope,
variable_source_info: SourceInfo, variable_source_info: SourceInfo,
declare_bindings: bool, declare_let_bindings: bool,
) -> BlockAnd<()> {
self.then_else_break_inner(
block,
expr_id,
ThenElseArgs {
temp_scope_override,
break_scope,
variable_source_info,
declare_let_bindings,
},
)
}
fn then_else_break_inner(
&mut self,
block: BasicBlock, // Block that the condition and branch will be lowered into
expr_id: ExprId, // Condition expression to lower
args: ThenElseArgs,
) -> BlockAnd<()> { ) -> BlockAnd<()> {
let this = self; let this = self;
let expr = &this.thir[expr_id]; let expr = &this.thir[expr_id];
@ -52,54 +86,36 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
match expr.kind { match expr.kind {
ExprKind::LogicalOp { op: LogicalOp::And, lhs, rhs } => { ExprKind::LogicalOp { op: LogicalOp::And, lhs, rhs } => {
let lhs_then_block = unpack!(this.then_else_break( let lhs_then_block = unpack!(this.then_else_break_inner(block, lhs, args));
block, let rhs_then_block = unpack!(this.then_else_break_inner(lhs_then_block, rhs, args));
lhs,
temp_scope_override,
break_scope,
variable_source_info,
declare_bindings,
));
let rhs_then_block = unpack!(this.then_else_break(
lhs_then_block,
rhs,
temp_scope_override,
break_scope,
variable_source_info,
declare_bindings,
));
rhs_then_block.unit() rhs_then_block.unit()
} }
ExprKind::LogicalOp { op: LogicalOp::Or, lhs, rhs } => { ExprKind::LogicalOp { op: LogicalOp::Or, lhs, rhs } => {
let local_scope = this.local_scope(); let local_scope = this.local_scope();
let (lhs_success_block, failure_block) = let (lhs_success_block, failure_block) =
this.in_if_then_scope(local_scope, expr_span, |this| { this.in_if_then_scope(local_scope, expr_span, |this| {
this.then_else_break( this.then_else_break_inner(
block, block,
lhs, lhs,
temp_scope_override, ThenElseArgs {
local_scope, break_scope: local_scope,
variable_source_info, declare_let_bindings: true,
true, ..args
},
) )
}); });
let rhs_success_block = unpack!(this.then_else_break( let rhs_success_block = unpack!(this.then_else_break_inner(
failure_block, failure_block,
rhs, rhs,
temp_scope_override, ThenElseArgs { declare_let_bindings: true, ..args },
break_scope,
variable_source_info,
true,
)); ));
// Make the LHS and RHS success arms converge to a common block. // Make the LHS and RHS success arms converge to a common block.
// (We can't just make LHS goto RHS, because `rhs_success_block` // (We can't just make LHS goto RHS, because `rhs_success_block`
// might contain statements that we don't want on the LHS path.) // might contain statements that we don't want on the LHS path.)
let success_block = this.cfg.start_new_block(); let success_block = this.cfg.start_new_block();
this.cfg.goto(lhs_success_block, variable_source_info, success_block); this.cfg.goto(lhs_success_block, args.variable_source_info, success_block);
this.cfg.goto(rhs_success_block, variable_source_info, success_block); this.cfg.goto(rhs_success_block, args.variable_source_info, success_block);
success_block.unit() success_block.unit()
} }
ExprKind::Unary { op: UnOp::Not, arg } => { ExprKind::Unary { op: UnOp::Not, arg } => {
@ -111,50 +127,38 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
if this.tcx.sess.instrument_coverage() { if this.tcx.sess.instrument_coverage() {
this.cfg.push_coverage_span_marker(block, this.source_info(expr_span)); this.cfg.push_coverage_span_marker(block, this.source_info(expr_span));
} }
this.then_else_break( this.then_else_break_inner(
block, block,
arg, arg,
temp_scope_override, ThenElseArgs {
local_scope, break_scope: local_scope,
variable_source_info, declare_let_bindings: true,
true, ..args
},
) )
}); });
this.break_for_else(success_block, break_scope, variable_source_info); this.break_for_else(success_block, args.break_scope, args.variable_source_info);
failure_block.unit() failure_block.unit()
} }
ExprKind::Scope { region_scope, lint_level, value } => { ExprKind::Scope { region_scope, lint_level, value } => {
let region_scope = (region_scope, this.source_info(expr_span)); let region_scope = (region_scope, this.source_info(expr_span));
this.in_scope(region_scope, lint_level, |this| { this.in_scope(region_scope, lint_level, |this| {
this.then_else_break( this.then_else_break_inner(block, value, args)
block,
value,
temp_scope_override,
break_scope,
variable_source_info,
declare_bindings,
)
}) })
} }
ExprKind::Use { source } => this.then_else_break( ExprKind::Use { source } => this.then_else_break_inner(block, source, args),
block,
source,
temp_scope_override,
break_scope,
variable_source_info,
declare_bindings,
),
ExprKind::Let { expr, ref pat } => this.lower_let_expr( ExprKind::Let { expr, ref pat } => this.lower_let_expr(
block, block,
expr, expr,
pat, pat,
break_scope, args.break_scope,
Some(variable_source_info.scope), Some(args.variable_source_info.scope),
variable_source_info.span, args.variable_source_info.span,
declare_bindings, args.declare_let_bindings,
), ),
_ => { _ => {
let temp_scope = temp_scope_override.unwrap_or_else(|| this.local_scope()); let mut block = block;
let temp_scope = args.temp_scope_override.unwrap_or_else(|| this.local_scope());
let mutability = Mutability::Mut; let mutability = Mutability::Mut;
let place = let place =
unpack!(block = this.as_temp(block, Some(temp_scope), expr_id, mutability)); unpack!(block = this.as_temp(block, Some(temp_scope), expr_id, mutability));
@ -166,7 +170,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
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, break_scope, source_info); this.break_for_else(else_block, args.break_scope, source_info);
then_block.unit() then_block.unit()
} }
@ -2105,10 +2109,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
this.then_else_break( this.then_else_break(
block, block,
guard, guard,
None, None, // Use `self.local_scope()` as the temp scope
match_scope, match_scope,
this.source_info(arm.span), this.source_info(arm.span),
false, false, // For guards, `let` bindings are declared separately
) )
}); });