Skip to content

Simplifications in match lowering #126835

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.

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

Merged
merged 8 commits into from
Jun 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 31 additions & 32 deletions compiler/rustc_mir_build/src/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,38 +189,37 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

let initializer_span = this.thir[*initializer].span;
let scope = (*init_scope, source_info);
let failure = unpack!(
block = this.in_scope(scope, *lint_level, |this| {
this.declare_bindings(
visibility_scope,
remainder_span,
pattern,
None,
Some((Some(&destination), initializer_span)),
);
this.visit_primary_bindings(
pattern,
UserTypeProjections::none(),
&mut |this, _, _, node, span, _, _| {
this.storage_live_binding(
block,
node,
span,
OutsideGuard,
true,
);
},
);
this.ast_let_else(
block,
*initializer,
initializer_span,
*else_block,
&last_remainder_scope,
pattern,
)
})
);
let failure_and_block = this.in_scope(scope, *lint_level, |this| {
this.declare_bindings(
visibility_scope,
remainder_span,
pattern,
None,
Some((Some(&destination), initializer_span)),
);
this.visit_primary_bindings(
pattern,
UserTypeProjections::none(),
&mut |this, _, _, node, span, _, _| {
this.storage_live_binding(block, node, span, OutsideGuard, true);
},
);
let else_block_span = this.thir[*else_block].span;
let (matching, failure) =
this.in_if_then_scope(last_remainder_scope, else_block_span, |this| {
this.lower_let_expr(
block,
*initializer,
pattern,
None,
initializer_span,
false,
true,
Comment on lines +216 to +217
Copy link
Contributor

@Zalathar Zalathar Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sort of thing makes me want to start adding dedicated enums to replace some of these magic booleans.

For example, I want to replace declare_let_bindings: bool with a three-way enum:

enum DeclareLetBindings {
    Yes,
    No,
    LetNotPermitted,
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've opened a draft PR to replace some booleans with enums: #126981.

)
});
matching.and(failure)
});
let failure = unpack!(block = failure_and_block);
this.cfg.goto(failure, source_info, failure_entry);

if let Some(source_scope) = visibility_scope {
Expand Down
73 changes: 18 additions & 55 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
Some(args.variable_source_info.scope),
args.variable_source_info.span,
args.declare_let_bindings,
false,
),
_ => {
let mut block = block;
Expand Down Expand Up @@ -1981,48 +1982,50 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

impl<'a, 'tcx> Builder<'a, 'tcx> {
/// If the bindings have already been declared, set `declare_bindings` to
/// `false` to avoid duplicated bindings declaration. Used for if-let guards.
/// `false` to avoid duplicated bindings declaration; used for if-let guards.
pub(crate) fn lower_let_expr(
&mut self,
mut block: BasicBlock,
expr_id: ExprId,
pat: &Pat<'tcx>,
source_scope: Option<SourceScope>,
span: Span,
scope_span: Span,
declare_bindings: bool,
storages_alive: bool,
) -> BlockAnd<()> {
let expr_span = self.thir[expr_id].span;
let expr_place_builder = unpack!(block = self.lower_scrutinee(block, expr_id, expr_span));
let mut guard_candidate = Candidate::new(expr_place_builder.clone(), pat, false, self);
let scrutinee = unpack!(block = self.lower_scrutinee(block, expr_id, expr_span));
let mut candidate = Candidate::new(scrutinee.clone(), pat, false, self);
let otherwise_block = self.lower_match_tree(
block,
expr_span,
&scrutinee,
pat.span,
&expr_place_builder,
pat.span,
&mut [&mut guard_candidate],
&mut [&mut candidate],
true,
);
let expr_place = expr_place_builder.try_to_place(self);
let opt_expr_place = expr_place.as_ref().map(|place| (Some(place), expr_span));

self.break_for_else(otherwise_block, self.source_info(expr_span));

if declare_bindings {
self.declare_bindings(source_scope, pat.span.to(span), pat, None, opt_expr_place);
let expr_place = scrutinee.try_to_place(self);
let opt_expr_place = expr_place.as_ref().map(|place| (Some(place), expr_span));
self.declare_bindings(source_scope, pat.span.to(scope_span), pat, None, opt_expr_place);
}

let post_guard_block = self.bind_pattern(
let success = self.bind_pattern(
self.source_info(pat.span),
guard_candidate,
candidate,
&[],
expr_span,
None,
false,
storages_alive,
);

// If branch coverage is enabled, record this branch.
self.visit_coverage_conditional_let(pat, post_guard_block, otherwise_block);
self.visit_coverage_conditional_let(pat, success, otherwise_block);

post_guard_block.unit()
success.unit()
}

/// Initializes each of the bindings from the candidate by
Expand Down Expand Up @@ -2469,44 +2472,4 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
debug!(?locals);
self.var_indices.insert(var_id, locals);
}

pub(crate) fn ast_let_else(
&mut self,
mut block: BasicBlock,
init_id: ExprId,
initializer_span: Span,
else_block: BlockId,
let_else_scope: &region::Scope,
pattern: &Pat<'tcx>,
) -> BlockAnd<BasicBlock> {
let else_block_span = self.thir[else_block].span;
let (matching, failure) = self.in_if_then_scope(*let_else_scope, else_block_span, |this| {
let scrutinee = unpack!(block = this.lower_scrutinee(block, init_id, initializer_span));
let mut candidate = Candidate::new(scrutinee.clone(), pattern, false, this);
let failure_block = this.lower_match_tree(
block,
initializer_span,
&scrutinee,
pattern.span,
&mut [&mut candidate],
true,
);
// This block is for the matching case
let matching = this.bind_pattern(
this.source_info(pattern.span),
candidate,
&[],
initializer_span,
None,
true,
);

// If branch coverage is enabled, record this branch.
this.visit_coverage_conditional_let(pattern, matching, failure_block);

this.break_for_else(failure_block, this.source_info(initializer_span));
matching.unit()
});
matching.and(failure)
}
}