Skip to content

Commit 3efbe83

Browse files
authored
Unrolled build for rust-lang#127472
Rollup merge of rust-lang#127472 - Zalathar:block-and-unit, r=fmease MIR building: Stop using `unpack!` for `BlockAnd<()>` This is a subset of rust-lang#127416, containing only the parts related to `BlockAnd<()>`. The first patch removes the non-assigning form of the `unpack!` macro, because it is frustratingly inconsistent with the main form. We can replace it with an ordinary method that discards the `()` and returns the block. The second patch then finds all of the remaining code that was using `unpack!` with `BlockAnd<()>`, and updates it to use that new method instead. --- Changes since original review of rust-lang#127416: - Renamed `fn unpack_block` → `fn into_block` - Removed `fn unpack_discard`, replacing it with `let _: BlockAnd<()> = ...` (2 occurrences) - Tweaked `arm_end_blocks` to unpack earlier and build `Vec<BasicBlock>` instead of `Vec<BlockAnd<()>>`
2 parents fcc325f + 1cf4eb2 commit 3efbe83

File tree

8 files changed

+86
-73
lines changed

8 files changed

+86
-73
lines changed

compiler/rustc_mir_build/src/build/block.rs

+13-13
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
7171
StmtKind::Expr { scope, expr } => {
7272
this.block_context.push(BlockFrame::Statement { ignores_expr_result: true });
7373
let si = (*scope, source_info);
74-
unpack!(
75-
block = this.in_scope(si, LintLevel::Inherited, |this| {
74+
block = this
75+
.in_scope(si, LintLevel::Inherited, |this| {
7676
this.stmt_expr(block, *expr, Some(*scope))
7777
})
78-
);
78+
.into_block();
7979
}
8080
StmtKind::Let {
8181
remainder_scope,
@@ -166,14 +166,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
166166
let dummy_place = this.temp(this.tcx.types.never, else_block_span);
167167
let failure_entry = this.cfg.start_new_block();
168168
let failure_block;
169-
unpack!(
170-
failure_block = this.ast_block(
169+
failure_block = this
170+
.ast_block(
171171
dummy_place,
172172
failure_entry,
173173
*else_block,
174174
this.source_info(else_block_span),
175175
)
176-
);
176+
.into_block();
177177
this.cfg.terminate(
178178
failure_block,
179179
this.source_info(else_block_span),
@@ -267,8 +267,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
267267
let initializer_span = this.thir[init].span;
268268
let scope = (*init_scope, source_info);
269269

270-
unpack!(
271-
block = this.in_scope(scope, *lint_level, |this| {
270+
block = this
271+
.in_scope(scope, *lint_level, |this| {
272272
this.declare_bindings(
273273
visibility_scope,
274274
remainder_span,
@@ -279,10 +279,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
279279
this.expr_into_pattern(block, &pattern, init)
280280
// irrefutable pattern
281281
})
282-
)
282+
.into_block();
283283
} else {
284284
let scope = (*init_scope, source_info);
285-
unpack!(this.in_scope(scope, *lint_level, |this| {
285+
let _: BlockAnd<()> = this.in_scope(scope, *lint_level, |this| {
286286
this.declare_bindings(
287287
visibility_scope,
288288
remainder_span,
@@ -291,7 +291,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
291291
None,
292292
);
293293
block.unit()
294-
}));
294+
});
295295

296296
debug!("ast_block_stmts: pattern={:?}", pattern);
297297
this.visit_primary_bindings(
@@ -333,7 +333,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
333333
this.block_context
334334
.push(BlockFrame::TailExpr { tail_result_is_ignored, span: expr.span });
335335

336-
unpack!(block = this.expr_into_dest(destination, block, expr_id));
336+
block = this.expr_into_dest(destination, block, expr_id).into_block();
337337
let popped = this.block_context.pop();
338338

339339
assert!(popped.is_some_and(|bf| bf.is_tail_expr()));
@@ -355,7 +355,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
355355
// Finally, we pop all the let scopes before exiting out from the scope of block
356356
// itself.
357357
for scope in let_scope_stack.into_iter().rev() {
358-
unpack!(block = this.pop_scope((*scope, source_info), block));
358+
block = this.pop_scope((*scope, source_info), block).into_block();
359359
}
360360
// Restore the original source scope.
361361
this.source_scope = outer_source_scope;

compiler/rustc_mir_build/src/build/expr/as_rvalue.rs

+4-8
Original file line numberDiff line numberDiff line change
@@ -185,13 +185,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
185185
this.cfg.push_assign(block, source_info, Place::from(result), box_);
186186

187187
// initialize the box contents:
188-
unpack!(
189-
block = this.expr_into_dest(
190-
this.tcx.mk_place_deref(Place::from(result)),
191-
block,
192-
value,
193-
)
194-
);
188+
block = this
189+
.expr_into_dest(this.tcx.mk_place_deref(Place::from(result)), block, value)
190+
.into_block();
195191
block.and(Rvalue::Use(Operand::Move(Place::from(result))))
196192
}
197193
ExprKind::Cast { source } => {
@@ -486,7 +482,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
486482
block.and(Rvalue::Aggregate(result, operands))
487483
}
488484
ExprKind::Assign { .. } | ExprKind::AssignOp { .. } => {
489-
block = unpack!(this.stmt_expr(block, expr_id, None));
485+
block = this.stmt_expr(block, expr_id, None).into_block();
490486
block.and(Rvalue::Use(Operand::Constant(Box::new(ConstOperand {
491487
span: expr_span,
492488
user_ty: None,

compiler/rustc_mir_build/src/build/expr/as_temp.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
112112
}
113113
}
114114

115-
unpack!(block = this.expr_into_dest(temp_place, block, expr_id));
115+
block = this.expr_into_dest(temp_place, block, expr_id).into_block();
116116

117117
if let Some(temp_lifetime) = temp_lifetime {
118118
this.schedule_drop(expr_span, temp_lifetime, temp, DropKind::Value);

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

+17-13
Original file line numberDiff line numberDiff line change
@@ -82,13 +82,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
8282
// Lower the condition, and have it branch into `then` and `else` blocks.
8383
let (then_block, else_block) =
8484
this.in_if_then_scope(condition_scope, then_span, |this| {
85-
let then_blk = unpack!(this.then_else_break(
86-
block,
87-
cond,
88-
Some(condition_scope), // Temp scope
89-
source_info,
90-
DeclareLetBindings::Yes, // Declare `let` bindings normally
91-
));
85+
let then_blk = this
86+
.then_else_break(
87+
block,
88+
cond,
89+
Some(condition_scope), // Temp scope
90+
source_info,
91+
DeclareLetBindings::Yes, // Declare `let` bindings normally
92+
)
93+
.into_block();
9294

9395
// Lower the `then` arm into its block.
9496
this.expr_into_dest(destination, then_blk, then)
@@ -105,7 +107,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
105107

106108
// If there is an `else` arm, lower it into `else_blk`.
107109
if let Some(else_expr) = else_opt {
108-
unpack!(else_blk = this.expr_into_dest(destination, else_blk, else_expr));
110+
else_blk = this.expr_into_dest(destination, else_blk, else_expr).into_block();
109111
} else {
110112
// There is no `else` arm, so we know both arms have type `()`.
111113
// Generate the implicit `else {}` by assigning unit.
@@ -187,7 +189,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
187189
const_: Const::from_bool(this.tcx, constant),
188190
},
189191
);
190-
let mut rhs_block = unpack!(this.expr_into_dest(destination, continuation, rhs));
192+
let mut rhs_block =
193+
this.expr_into_dest(destination, continuation, rhs).into_block();
191194
// Instrument the lowered RHS's value for condition coverage.
192195
// (Does nothing if condition coverage is not enabled.)
193196
this.visit_coverage_standalone_condition(rhs, destination, &mut rhs_block);
@@ -230,7 +233,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
230233
// introduce a unit temporary as the destination for the loop body.
231234
let tmp = this.get_unit_temp();
232235
// Execute the body, branching back to the test.
233-
let body_block_end = unpack!(this.expr_into_dest(tmp, body_block, body));
236+
let body_block_end = this.expr_into_dest(tmp, body_block, body).into_block();
234237
this.cfg.goto(body_block_end, source_info, loop_block);
235238

236239
// Loops are only exited by `break` expressions.
@@ -462,7 +465,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
462465
targets.push(target);
463466

464467
let tmp = this.get_unit_temp();
465-
let target = unpack!(this.ast_block(tmp, target, block, source_info));
468+
let target =
469+
this.ast_block(tmp, target, block, source_info).into_block();
466470
this.cfg.terminate(
467471
target,
468472
source_info,
@@ -502,7 +506,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
502506

503507
// These cases don't actually need a destination
504508
ExprKind::Assign { .. } | ExprKind::AssignOp { .. } => {
505-
unpack!(block = this.stmt_expr(block, expr_id, None));
509+
block = this.stmt_expr(block, expr_id, None).into_block();
506510
this.cfg.push_assign_unit(block, source_info, destination, this.tcx);
507511
block.unit()
508512
}
@@ -511,7 +515,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
511515
| ExprKind::Break { .. }
512516
| ExprKind::Return { .. }
513517
| ExprKind::Become { .. } => {
514-
unpack!(block = this.stmt_expr(block, expr_id, None));
518+
block = this.stmt_expr(block, expr_id, None).into_block();
515519
// No assign, as these have type `!`.
516520
block.unit()
517521
}

compiler/rustc_mir_build/src/build/expr/stmt.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
4444
if lhs_expr.ty.needs_drop(this.tcx, this.param_env) {
4545
let rhs = unpack!(block = this.as_local_rvalue(block, rhs));
4646
let lhs = unpack!(block = this.as_place(block, lhs));
47-
unpack!(block = this.build_drop_and_replace(block, lhs_expr.span, lhs, rhs));
47+
block =
48+
this.build_drop_and_replace(block, lhs_expr.span, lhs, rhs).into_block();
4849
} else {
4950
let rhs = unpack!(block = this.as_local_rvalue(block, rhs));
5051
let lhs = unpack!(block = this.as_place(block, lhs));

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

+19-15
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
122122
match expr.kind {
123123
ExprKind::LogicalOp { op: op @ LogicalOp::And, lhs, rhs } => {
124124
this.visit_coverage_branch_operation(op, expr_span);
125-
let lhs_then_block = unpack!(this.then_else_break_inner(block, lhs, args));
126-
let rhs_then_block = unpack!(this.then_else_break_inner(lhs_then_block, rhs, args));
125+
let lhs_then_block = this.then_else_break_inner(block, lhs, args).into_block();
126+
let rhs_then_block =
127+
this.then_else_break_inner(lhs_then_block, rhs, args).into_block();
127128
rhs_then_block.unit()
128129
}
129130
ExprKind::LogicalOp { op: op @ LogicalOp::Or, lhs, rhs } => {
@@ -140,14 +141,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
140141
},
141142
)
142143
});
143-
let rhs_success_block = unpack!(this.then_else_break_inner(
144-
failure_block,
145-
rhs,
146-
ThenElseArgs {
147-
declare_let_bindings: DeclareLetBindings::LetNotPermitted,
148-
..args
149-
},
150-
));
144+
let rhs_success_block = this
145+
.then_else_break_inner(
146+
failure_block,
147+
rhs,
148+
ThenElseArgs {
149+
declare_let_bindings: DeclareLetBindings::LetNotPermitted,
150+
..args
151+
},
152+
)
153+
.into_block();
151154

152155
// Make the LHS and RHS success arms converge to a common block.
153156
// (We can't just make LHS goto RHS, because `rhs_success_block`
@@ -452,7 +455,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
452455
outer_source_info: SourceInfo,
453456
fake_borrow_temps: Vec<(Place<'tcx>, Local, FakeBorrowKind)>,
454457
) -> BlockAnd<()> {
455-
let arm_end_blocks: Vec<_> = arm_candidates
458+
let arm_end_blocks: Vec<BasicBlock> = arm_candidates
456459
.into_iter()
457460
.map(|(arm, candidate)| {
458461
debug!("lowering arm {:?}\ncandidate = {:?}", arm, candidate);
@@ -503,6 +506,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
503506

504507
this.expr_into_dest(destination, arm_block, arm.body)
505508
})
509+
.into_block()
506510
})
507511
.collect();
508512

@@ -513,10 +517,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
513517
outer_source_info.span.with_lo(outer_source_info.span.hi() - BytePos::from_usize(1)),
514518
);
515519
for arm_block in arm_end_blocks {
516-
let block = &self.cfg.basic_blocks[arm_block.0];
520+
let block = &self.cfg.basic_blocks[arm_block];
517521
let last_location = block.statements.last().map(|s| s.source_info);
518522

519-
self.cfg.goto(unpack!(arm_block), last_location.unwrap_or(end_brace), end_block);
523+
self.cfg.goto(arm_block, last_location.unwrap_or(end_brace), end_block);
520524
}
521525

522526
self.source_scope = outer_source_info.scope;
@@ -622,7 +626,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
622626
OutsideGuard,
623627
ScheduleDrops::Yes,
624628
);
625-
unpack!(block = self.expr_into_dest(place, block, initializer_id));
629+
block = self.expr_into_dest(place, block, initializer_id).into_block();
626630

627631
// Inject a fake read, see comments on `FakeReadCause::ForLet`.
628632
let source_info = self.source_info(irrefutable_pat.span);
@@ -661,7 +665,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
661665
OutsideGuard,
662666
ScheduleDrops::Yes,
663667
);
664-
unpack!(block = self.expr_into_dest(place, block, initializer_id));
668+
block = self.expr_into_dest(place, block, initializer_id).into_block();
665669

666670
// Inject a fake read, see comments on `FakeReadCause::ForLet`.
667671
let pattern_source_info = self.source_info(irrefutable_pat.span);

compiler/rustc_mir_build/src/build/mod.rs

+17-12
Original file line numberDiff line numberDiff line change
@@ -403,6 +403,15 @@ enum NeedsTemporary {
403403
#[must_use = "if you don't use one of these results, you're leaving a dangling edge"]
404404
struct BlockAnd<T>(BasicBlock, T);
405405

406+
impl BlockAnd<()> {
407+
/// Unpacks `BlockAnd<()>` into a [`BasicBlock`].
408+
#[must_use]
409+
fn into_block(self) -> BasicBlock {
410+
let Self(block, ()) = self;
411+
block
412+
}
413+
}
414+
406415
trait BlockAndExtension {
407416
fn and<T>(self, v: T) -> BlockAnd<T>;
408417
fn unit(self) -> BlockAnd<()>;
@@ -426,11 +435,6 @@ macro_rules! unpack {
426435
$x = b;
427436
v
428437
}};
429-
430-
($c:expr) => {{
431-
let BlockAnd(b, ()) = $c;
432-
b
433-
}};
434438
}
435439

436440
///////////////////////////////////////////////////////////////////////////
@@ -516,21 +520,22 @@ fn construct_fn<'tcx>(
516520
region::Scope { id: body.id().hir_id.local_id, data: region::ScopeData::Arguments };
517521
let source_info = builder.source_info(span);
518522
let call_site_s = (call_site_scope, source_info);
519-
unpack!(builder.in_scope(call_site_s, LintLevel::Inherited, |builder| {
523+
let _: BlockAnd<()> = builder.in_scope(call_site_s, LintLevel::Inherited, |builder| {
520524
let arg_scope_s = (arg_scope, source_info);
521525
// Attribute epilogue to function's closing brace
522526
let fn_end = span_with_body.shrink_to_hi();
523-
let return_block =
524-
unpack!(builder.in_breakable_scope(None, Place::return_place(), fn_end, |builder| {
527+
let return_block = builder
528+
.in_breakable_scope(None, Place::return_place(), fn_end, |builder| {
525529
Some(builder.in_scope(arg_scope_s, LintLevel::Inherited, |builder| {
526530
builder.args_and_body(START_BLOCK, arguments, arg_scope, expr)
527531
}))
528-
}));
532+
})
533+
.into_block();
529534
let source_info = builder.source_info(fn_end);
530535
builder.cfg.terminate(return_block, source_info, TerminatorKind::Return);
531536
builder.build_drop_trees();
532537
return_block.unit()
533-
}));
538+
});
534539

535540
let mut body = builder.finish();
536541

@@ -579,7 +584,7 @@ fn construct_const<'a, 'tcx>(
579584
Builder::new(thir, infcx, def, hir_id, span, 0, const_ty, const_ty_span, None);
580585

581586
let mut block = START_BLOCK;
582-
unpack!(block = builder.expr_into_dest(Place::return_place(), block, expr));
587+
block = builder.expr_into_dest(Place::return_place(), block, expr).into_block();
583588

584589
let source_info = builder.source_info(span);
585590
builder.cfg.terminate(block, source_info, TerminatorKind::Return);
@@ -961,7 +966,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
961966
Some((Some(&place), span)),
962967
);
963968
let place_builder = PlaceBuilder::from(local);
964-
unpack!(block = self.place_into_pattern(block, pat, place_builder, false));
969+
block = self.place_into_pattern(block, pat, place_builder, false).into_block();
965970
}
966971
}
967972
self.source_scope = original_source_scope;

0 commit comments

Comments
 (0)