Skip to content

Commit 5fd72c5

Browse files
rust-timerrust-timer
authored
Unrolled build for rust-lang#122137
Rollup merge of rust-lang#122137 - Zalathar:if-break-scope, r=matthewjasper 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. --- Both ways of indicating the break target were introduced in rust-lang#88572. I haven't been able to find any strong indication of whether this was done deliberately, or whether it was just an implementation artifact. But to me it doesn't seem useful to carefully pass around the same scope in two different ways.
2 parents 9c3ad80 + 570376c commit 5fd72c5

File tree

3 files changed

+19
-38
lines changed

3 files changed

+19
-38
lines changed

‎compiler/rustc_mir_build/src/build/expr/into.rs

-2
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
8383
block,
8484
cond,
8585
Some(condition_scope), // Temp scope
86-
condition_scope,
8786
source_info,
8887
true, // Declare `let` bindings normally
8988
));
@@ -156,7 +155,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
156155
block,
157156
lhs,
158157
Some(condition_scope), // Temp scope
159-
condition_scope,
160158
source_info,
161159
// This flag controls how inner `let` expressions are lowered,
162160
// but either way there shouldn't be any of those in here.

‎compiler/rustc_mir_build/src/build/matches/mod.rs

+7-27
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,6 @@ struct ThenElseArgs {
3737
/// Used as the temp scope for lowering `expr`. If absent (for match guards),
3838
/// `self.local_scope()` is used.
3939
temp_scope_override: Option<region::Scope>,
40-
/// Scope to pass to [`Builder::break_for_else`]. Must match the scope used
41-
/// by the enclosing call to [`Builder::in_if_then_scope`].
42-
break_scope: region::Scope,
4340
variable_source_info: SourceInfo,
4441
/// Forwarded to [`Builder::lower_let_expr`] when lowering [`ExprKind::Let`].
4542
/// When false (for match guards), `let` bindings won't be declared.
@@ -58,19 +55,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
5855
block: BasicBlock,
5956
expr_id: ExprId,
6057
temp_scope_override: Option<region::Scope>,
61-
break_scope: region::Scope,
6258
variable_source_info: SourceInfo,
6359
declare_let_bindings: bool,
6460
) -> BlockAnd<()> {
6561
self.then_else_break_inner(
6662
block,
6763
expr_id,
68-
ThenElseArgs {
69-
temp_scope_override,
70-
break_scope,
71-
variable_source_info,
72-
declare_let_bindings,
73-
},
64+
ThenElseArgs { temp_scope_override, variable_source_info, declare_let_bindings },
7465
)
7566
}
7667

@@ -97,11 +88,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
9788
this.then_else_break_inner(
9889
block,
9990
lhs,
100-
ThenElseArgs {
101-
break_scope: local_scope,
102-
declare_let_bindings: true,
103-
..args
104-
},
91+
ThenElseArgs { declare_let_bindings: true, ..args },
10592
)
10693
});
10794
let rhs_success_block = unpack!(this.then_else_break_inner(
@@ -130,14 +117,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
130117
this.then_else_break_inner(
131118
block,
132119
arg,
133-
ThenElseArgs {
134-
break_scope: local_scope,
135-
declare_let_bindings: true,
136-
..args
137-
},
120+
ThenElseArgs { declare_let_bindings: true, ..args },
138121
)
139122
});
140-
this.break_for_else(success_block, args.break_scope, args.variable_source_info);
123+
this.break_for_else(success_block, args.variable_source_info);
141124
failure_block.unit()
142125
}
143126
ExprKind::Scope { region_scope, lint_level, value } => {
@@ -151,7 +134,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
151134
block,
152135
expr,
153136
pat,
154-
args.break_scope,
155137
Some(args.variable_source_info.scope),
156138
args.variable_source_info.span,
157139
args.declare_let_bindings,
@@ -170,7 +152,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
170152

171153
let source_info = this.source_info(expr_span);
172154
this.cfg.terminate(block, source_info, term);
173-
this.break_for_else(else_block, args.break_scope, source_info);
155+
this.break_for_else(else_block, source_info);
174156

175157
then_block.unit()
176158
}
@@ -1911,7 +1893,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
19111893
mut block: BasicBlock,
19121894
expr_id: ExprId,
19131895
pat: &Pat<'tcx>,
1914-
else_target: region::Scope,
19151896
source_scope: Option<SourceScope>,
19161897
span: Span,
19171898
declare_bindings: bool,
@@ -1933,7 +1914,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
19331914
let expr_place = expr_place_builder.try_to_place(self);
19341915
let opt_expr_place = expr_place.as_ref().map(|place| (Some(place), expr_span));
19351916
let otherwise_post_guard_block = otherwise_candidate.pre_binding_block.unwrap();
1936-
self.break_for_else(otherwise_post_guard_block, else_target, self.source_info(expr_span));
1917+
self.break_for_else(otherwise_post_guard_block, self.source_info(expr_span));
19371918

19381919
if declare_bindings {
19391920
self.declare_bindings(source_scope, pat.span.to(span), pat, None, opt_expr_place);
@@ -2110,7 +2091,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
21102091
block,
21112092
guard,
21122093
None, // Use `self.local_scope()` as the temp scope
2113-
match_scope,
21142094
this.source_info(arm.span),
21152095
false, // For guards, `let` bindings are declared separately
21162096
)
@@ -2443,7 +2423,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
24432423
None,
24442424
true,
24452425
);
2446-
this.break_for_else(failure, *let_else_scope, this.source_info(initializer_span));
2426+
this.break_for_else(failure, this.source_info(initializer_span));
24472427
matching.unit()
24482428
});
24492429
matching.and(failure)

‎compiler/rustc_mir_build/src/build/scope.rs

+12-9
Original file line numberDiff line numberDiff line change
@@ -683,20 +683,23 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
683683
self.cfg.start_new_block().unit()
684684
}
685685

686-
pub(crate) fn break_for_else(
687-
&mut self,
688-
block: BasicBlock,
689-
target: region::Scope,
690-
source_info: SourceInfo,
691-
) {
692-
let scope_index = self.scopes.scope_index(target, source_info.span);
686+
/// Sets up the drops for breaking from `block` due to an `if` condition
687+
/// that turned out to be false.
688+
///
689+
/// Must be called in the context of [`Builder::in_if_then_scope`], so that
690+
/// there is an if-then scope to tell us what the target scope is.
691+
pub(crate) fn break_for_else(&mut self, block: BasicBlock, source_info: SourceInfo) {
693692
let if_then_scope = self
694693
.scopes
695694
.if_then_scope
696-
.as_mut()
695+
.as_ref()
697696
.unwrap_or_else(|| span_bug!(source_info.span, "no if-then scope found"));
698697

699-
assert_eq!(if_then_scope.region_scope, target, "breaking to incorrect scope");
698+
let target = if_then_scope.region_scope;
699+
let scope_index = self.scopes.scope_index(target, source_info.span);
700+
701+
// Upgrade `if_then_scope` to `&mut`.
702+
let if_then_scope = self.scopes.if_then_scope.as_mut().expect("upgrading & to &mut");
700703

701704
let mut drop_idx = ROOT_NODE;
702705
let drops = &mut if_then_scope.else_drops;

0 commit comments

Comments
 (0)