Skip to content

Commit

Permalink
Don't pass a break scope to Builder::break_for_else
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Zalathar committed Mar 7, 2024
1 parent 51f4839 commit 570376c
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 38 deletions.
2 changes: 0 additions & 2 deletions compiler/rustc_mir_build/src/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
block,
cond,
Some(condition_scope), // Temp scope
condition_scope,
source_info,
true, // Declare `let` bindings normally
));
Expand Down Expand Up @@ -156,7 +155,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
block,
lhs,
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.
Expand Down
34 changes: 7 additions & 27 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@ 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.
Expand All @@ -58,19 +55,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
block: BasicBlock,
expr_id: ExprId,
temp_scope_override: Option<region::Scope>,
break_scope: region::Scope,
variable_source_info: SourceInfo,
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,
},
ThenElseArgs { temp_scope_override, variable_source_info, declare_let_bindings },
)
}

Expand All @@ -97,11 +88,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
this.then_else_break_inner(
block,
lhs,
ThenElseArgs {
break_scope: local_scope,
declare_let_bindings: true,
..args
},
ThenElseArgs { declare_let_bindings: true, ..args },
)
});
let rhs_success_block = unpack!(this.then_else_break_inner(
Expand Down Expand Up @@ -130,14 +117,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
this.then_else_break_inner(
block,
arg,
ThenElseArgs {
break_scope: local_scope,
declare_let_bindings: true,
..args
},
ThenElseArgs { 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()
}
ExprKind::Scope { region_scope, lint_level, value } => {
Expand All @@ -151,7 +134,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
block,
expr,
pat,
args.break_scope,
Some(args.variable_source_info.scope),
args.variable_source_info.span,
args.declare_let_bindings,
Expand All @@ -170,7 +152,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, args.break_scope, source_info);
this.break_for_else(else_block, source_info);

then_block.unit()
}
Expand Down Expand Up @@ -1911,7 +1893,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
mut block: BasicBlock,
expr_id: ExprId,
pat: &Pat<'tcx>,
else_target: region::Scope,
source_scope: Option<SourceScope>,
span: Span,
declare_bindings: bool,
Expand All @@ -1933,7 +1914,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
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 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 {
self.declare_bindings(source_scope, pat.span.to(span), pat, None, opt_expr_place);
Expand Down Expand Up @@ -2110,7 +2091,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
block,
guard,
None, // Use `self.local_scope()` as the temp scope
match_scope,
this.source_info(arm.span),
false, // For guards, `let` bindings are declared separately
)
Expand Down Expand Up @@ -2443,7 +2423,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
None,
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.and(failure)
Expand Down
21 changes: 12 additions & 9 deletions compiler/rustc_mir_build/src/build/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -683,20 +683,23 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.cfg.start_new_block().unit()
}

pub(crate) fn break_for_else(
&mut self,
block: BasicBlock,
target: region::Scope,
source_info: SourceInfo,
) {
let scope_index = self.scopes.scope_index(target, source_info.span);
/// Sets up the drops for breaking from `block` due to an `if` condition
/// that turned out to be false.
///
/// Must be called in the context of [`Builder::in_if_then_scope`], so that
/// 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 if_then_scope = self
.scopes
.if_then_scope
.as_mut()
.as_ref()
.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 drops = &mut if_then_scope.else_drops;
Expand Down

0 comments on commit 570376c

Please sign in to comment.