Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Extract an arguments struct for Builder::then_else_break #121928

Merged
merged 1 commit into from Mar 5, 2024
+72 −66
Merged

Extract an arguments struct for Builder::then_else_break #121928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 5 additions & 3 deletions compiler/rustc_mir_build/src/build/expr/into.rs
View file
Open in desktop
Expand Up @@ -81,10 +81,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let then_blk = unpack!(this.then_else_break(
block,
cond,
Some(condition_scope),
Some(condition_scope), // Temp scope
condition_scope,
source_info,
true,
true, // Declare `let` bindings normally
));

this.expr_into_dest(destination, then_blk, then)
Expand Down Expand Up @@ -146,9 +146,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
this.then_else_break(
block,
lhs,
Some(condition_scope),
Some(condition_scope), // Temp scope
condition_scope,
source_info,
// This flag controls how inner `let` expressions are lowered,
// but either way there shouldn't be any of those in here.
true,
)
});
Expand Down
130 changes: 67 additions & 63 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
View file
Open in desktop
Expand Up @@ -30,76 +30,92 @@ mod util;
use std::borrow::Borrow;
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> {
/// Lowers a condition in a way that ensures that variables bound in any let
/// 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
/// an or pattern, where the guard is lowered multiple times.
pub(crate) fn then_else_break(
&mut self,
mut block: BasicBlock,
block: BasicBlock,
expr_id: ExprId,
temp_scope_override: Option<region::Scope>,
break_scope: region::Scope,
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<()> {
let this = self;
let expr = &this.thir[expr_id];
let expr_span = expr.span;

match expr.kind {
ExprKind::LogicalOp { op: LogicalOp::And, lhs, rhs } => {
let lhs_then_block = unpack!(this.then_else_break(
block,
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,
));

let lhs_then_block = unpack!(this.then_else_break_inner(block, lhs, args));
let rhs_then_block = unpack!(this.then_else_break_inner(lhs_then_block, rhs, args));
rhs_then_block.unit()
}
ExprKind::LogicalOp { op: LogicalOp::Or, lhs, rhs } => {
let local_scope = this.local_scope();
let (lhs_success_block, failure_block) =
this.in_if_then_scope(local_scope, expr_span, |this| {
this.then_else_break(
this.then_else_break_inner(
block,
lhs,
temp_scope_override,
local_scope,
variable_source_info,
true,
ThenElseArgs {
break_scope: local_scope,
declare_let_bindings: true,
..args
},
)
});
let rhs_success_block = unpack!(this.then_else_break(
let rhs_success_block = unpack!(this.then_else_break_inner(
failure_block,
rhs,
temp_scope_override,
break_scope,
variable_source_info,
true,
ThenElseArgs { declare_let_bindings: true, ..args },
));

// Make the LHS and RHS success arms converge to a common 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.)
let success_block = this.cfg.start_new_block();
this.cfg.goto(lhs_success_block, variable_source_info, success_block);
this.cfg.goto(rhs_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, args.variable_source_info, success_block);
success_block.unit()
}
ExprKind::Unary { op: UnOp::Not, arg } => {
Expand All @@ -111,50 +127,38 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
if this.tcx.sess.instrument_coverage() {
this.cfg.push_coverage_span_marker(block, this.source_info(expr_span));
}
this.then_else_break(
this.then_else_break_inner(
block,
arg,
temp_scope_override,
local_scope,
variable_source_info,
true,
ThenElseArgs {
break_scope: local_scope,
declare_let_bindings: 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()
}
ExprKind::Scope { region_scope, lint_level, value } => {
let region_scope = (region_scope, this.source_info(expr_span));
this.in_scope(region_scope, lint_level, |this| {
this.then_else_break(
block,
value,
temp_scope_override,
break_scope,
variable_source_info,
declare_bindings,
)
this.then_else_break_inner(block, value, args)
})
}
ExprKind::Use { source } => this.then_else_break(
block,
source,
temp_scope_override,
break_scope,
variable_source_info,
declare_bindings,
),
ExprKind::Use { source } => this.then_else_break_inner(block, source, args),
ExprKind::Let { expr, ref pat } => this.lower_let_expr(
block,
expr,
pat,
break_scope,
Some(variable_source_info.scope),
variable_source_info.span,
declare_bindings,
args.break_scope,
Some(args.variable_source_info.scope),
args.variable_source_info.span,
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 place =
unpack!(block = this.as_temp(block, Some(temp_scope), expr_id, mutability));
Expand All @@ -166,7 +170,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

let source_info = this.source_info(expr_span);
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()
}
Expand Down Expand Up @@ -2105,10 +2109,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
this.then_else_break(
block,
guard,
None,
None, // Use `self.local_scope()` as the temp scope
match_scope,
this.source_info(arm.span),
false,
false, // For guards, `let` bindings are declared separately
)
});

Expand Down