From 4fe8dd05ed844df0509adcad801e8ff32716c2f8 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 6 Jul 2024 11:45:47 +1000 Subject: [PATCH 1/2] Remove the non-assigning form of `unpack!` This kind of unpacking can be expressed as an ordinary method on `BlockAnd<()>`. --- compiler/rustc_mir_build/src/build/block.rs | 4 +-- .../src/build/expr/as_rvalue.rs | 2 +- .../rustc_mir_build/src/build/expr/into.rs | 24 ++++++++------- .../rustc_mir_build/src/build/matches/mod.rs | 30 +++++++++++-------- compiler/rustc_mir_build/src/build/mod.rs | 25 +++++++++------- compiler/rustc_mir_build/src/build/scope.rs | 19 +++++++----- 6 files changed, 60 insertions(+), 44 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/block.rs b/compiler/rustc_mir_build/src/build/block.rs index 5ccbd7c59cfba..813fd17606daf 100644 --- a/compiler/rustc_mir_build/src/build/block.rs +++ b/compiler/rustc_mir_build/src/build/block.rs @@ -282,7 +282,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ) } else { let scope = (*init_scope, source_info); - unpack!(this.in_scope(scope, *lint_level, |this| { + let _: BlockAnd<()> = this.in_scope(scope, *lint_level, |this| { this.declare_bindings( visibility_scope, remainder_span, @@ -291,7 +291,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { None, ); block.unit() - })); + }); debug!("ast_block_stmts: pattern={:?}", pattern); this.visit_primary_bindings( diff --git a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs index c5ee6db5999a5..f0725e18b46e2 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -486,7 +486,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block.and(Rvalue::Aggregate(result, operands)) } ExprKind::Assign { .. } | ExprKind::AssignOp { .. } => { - block = unpack!(this.stmt_expr(block, expr_id, None)); + block = this.stmt_expr(block, expr_id, None).into_block(); block.and(Rvalue::Use(Operand::Constant(Box::new(ConstOperand { span: expr_span, user_ty: None, diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index 942c69b5c0a75..0e5337ba4e12d 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -82,13 +82,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Lower the condition, and have it branch into `then` and `else` blocks. let (then_block, else_block) = this.in_if_then_scope(condition_scope, then_span, |this| { - let then_blk = unpack!(this.then_else_break( - block, - cond, - Some(condition_scope), // Temp scope - source_info, - DeclareLetBindings::Yes, // Declare `let` bindings normally - )); + let then_blk = this + .then_else_break( + block, + cond, + Some(condition_scope), // Temp scope + source_info, + DeclareLetBindings::Yes, // Declare `let` bindings normally + ) + .into_block(); // Lower the `then` arm into its block. this.expr_into_dest(destination, then_blk, then) @@ -187,7 +189,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { const_: Const::from_bool(this.tcx, constant), }, ); - let mut rhs_block = unpack!(this.expr_into_dest(destination, continuation, rhs)); + let mut rhs_block = + this.expr_into_dest(destination, continuation, rhs).into_block(); // Instrument the lowered RHS's value for condition coverage. // (Does nothing if condition coverage is not enabled.) this.visit_coverage_standalone_condition(rhs, destination, &mut rhs_block); @@ -230,7 +233,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // introduce a unit temporary as the destination for the loop body. let tmp = this.get_unit_temp(); // Execute the body, branching back to the test. - let body_block_end = unpack!(this.expr_into_dest(tmp, body_block, body)); + let body_block_end = this.expr_into_dest(tmp, body_block, body).into_block(); this.cfg.goto(body_block_end, source_info, loop_block); // Loops are only exited by `break` expressions. @@ -462,7 +465,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { targets.push(target); let tmp = this.get_unit_temp(); - let target = unpack!(this.ast_block(tmp, target, block, source_info)); + let target = + this.ast_block(tmp, target, block, source_info).into_block(); this.cfg.terminate( target, source_info, diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index e435e2f92883c..be1ea0f8ba9a4 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -121,8 +121,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { match expr.kind { ExprKind::LogicalOp { op: op @ LogicalOp::And, lhs, rhs } => { this.visit_coverage_branch_operation(op, expr_span); - 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)); + let lhs_then_block = this.then_else_break_inner(block, lhs, args).into_block(); + let rhs_then_block = + this.then_else_break_inner(lhs_then_block, rhs, args).into_block(); rhs_then_block.unit() } ExprKind::LogicalOp { op: op @ LogicalOp::Or, lhs, rhs } => { @@ -139,14 +140,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }, ) }); - let rhs_success_block = unpack!(this.then_else_break_inner( - failure_block, - rhs, - ThenElseArgs { - declare_let_bindings: DeclareLetBindings::LetNotPermitted, - ..args - }, - )); + let rhs_success_block = this + .then_else_break_inner( + failure_block, + rhs, + ThenElseArgs { + declare_let_bindings: DeclareLetBindings::LetNotPermitted, + ..args + }, + ) + .into_block(); // Make the LHS and RHS success arms converge to a common block. // (We can't just make LHS goto RHS, because `rhs_success_block` @@ -451,7 +454,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { outer_source_info: SourceInfo, fake_borrow_temps: Vec<(Place<'tcx>, Local, FakeBorrowKind)>, ) -> BlockAnd<()> { - let arm_end_blocks: Vec<_> = arm_candidates + let arm_end_blocks: Vec = arm_candidates .into_iter() .map(|(arm, candidate)| { debug!("lowering arm {:?}\ncandidate = {:?}", arm, candidate); @@ -502,6 +505,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.expr_into_dest(destination, arm_block, arm.body) }) + .into_block() }) .collect(); @@ -512,10 +516,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { outer_source_info.span.with_lo(outer_source_info.span.hi() - BytePos::from_usize(1)), ); for arm_block in arm_end_blocks { - let block = &self.cfg.basic_blocks[arm_block.0]; + let block = &self.cfg.basic_blocks[arm_block]; let last_location = block.statements.last().map(|s| s.source_info); - self.cfg.goto(unpack!(arm_block), last_location.unwrap_or(end_brace), end_block); + self.cfg.goto(arm_block, last_location.unwrap_or(end_brace), end_block); } self.source_scope = outer_source_info.scope; diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index 0f9746cb719ca..04aaa806d5946 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -403,6 +403,15 @@ enum NeedsTemporary { #[must_use = "if you don't use one of these results, you're leaving a dangling edge"] struct BlockAnd(BasicBlock, T); +impl BlockAnd<()> { + /// Unpacks `BlockAnd<()>` into a [`BasicBlock`]. + #[must_use] + fn into_block(self) -> BasicBlock { + let Self(block, ()) = self; + block + } +} + trait BlockAndExtension { fn and(self, v: T) -> BlockAnd; fn unit(self) -> BlockAnd<()>; @@ -426,11 +435,6 @@ macro_rules! unpack { $x = b; v }}; - - ($c:expr) => {{ - let BlockAnd(b, ()) = $c; - b - }}; } /////////////////////////////////////////////////////////////////////////// @@ -516,21 +520,22 @@ fn construct_fn<'tcx>( region::Scope { id: body.id().hir_id.local_id, data: region::ScopeData::Arguments }; let source_info = builder.source_info(span); let call_site_s = (call_site_scope, source_info); - unpack!(builder.in_scope(call_site_s, LintLevel::Inherited, |builder| { + let _: BlockAnd<()> = builder.in_scope(call_site_s, LintLevel::Inherited, |builder| { let arg_scope_s = (arg_scope, source_info); // Attribute epilogue to function's closing brace let fn_end = span_with_body.shrink_to_hi(); - let return_block = - unpack!(builder.in_breakable_scope(None, Place::return_place(), fn_end, |builder| { + let return_block = builder + .in_breakable_scope(None, Place::return_place(), fn_end, |builder| { Some(builder.in_scope(arg_scope_s, LintLevel::Inherited, |builder| { builder.args_and_body(START_BLOCK, arguments, arg_scope, expr) })) - })); + }) + .into_block(); let source_info = builder.source_info(fn_end); builder.cfg.terminate(return_block, source_info, TerminatorKind::Return); builder.build_drop_trees(); return_block.unit() - })); + }); let mut body = builder.finish(); diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index 5b6de39bb2e78..d6be0e7a252e3 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -510,12 +510,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let target = self.cfg.start_new_block(); let source_info = self.source_info(span); self.cfg.terminate( - unpack!(normal_block), + normal_block.into_block(), source_info, TerminatorKind::Goto { target }, ); self.cfg.terminate( - unpack!(exit_block), + exit_block.into_block(), source_info, TerminatorKind::Goto { target }, ); @@ -552,14 +552,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let scope = IfThenScope { region_scope, else_drops: DropTree::new() }; let previous_scope = mem::replace(&mut self.scopes.if_then_scope, Some(scope)); - let then_block = unpack!(f(self)); + let then_block = f(self).into_block(); let if_then_scope = mem::replace(&mut self.scopes.if_then_scope, previous_scope).unwrap(); assert!(if_then_scope.region_scope == region_scope); - let else_block = self - .build_exit_tree(if_then_scope.else_drops, region_scope, span, None) - .map_or_else(|| self.cfg.start_new_block(), |else_block_and| unpack!(else_block_and)); + let else_block = + self.build_exit_tree(if_then_scope.else_drops, region_scope, span, None).map_or_else( + || self.cfg.start_new_block(), + |else_block_and| else_block_and.into_block(), + ); (then_block, else_block) } @@ -753,7 +755,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let unwind_to = if needs_cleanup { self.diverge_cleanup() } else { DropIdx::MAX }; let scope = self.scopes.scopes.last().expect("leave_top_scope called with no scopes"); - unpack!(build_scope_drops( + build_scope_drops( &mut self.cfg, &mut self.scopes.unwind_drops, scope, @@ -761,7 +763,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { unwind_to, is_coroutine && needs_cleanup, self.arg_count, - )) + ) + .into_block() } /// Possibly creates a new source scope if `current_root` and `parent_root` From 1cf4eb2ad258281fcf59ec93f41b43117bb5b824 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 6 Jul 2024 11:55:54 +1000 Subject: [PATCH 2/2] Stop using `unpack!` for `BlockAnd<()>` --- compiler/rustc_mir_build/src/build/block.rs | 22 +++++++++---------- .../src/build/expr/as_rvalue.rs | 10 +++------ .../rustc_mir_build/src/build/expr/as_temp.rs | 2 +- .../rustc_mir_build/src/build/expr/into.rs | 6 ++--- .../rustc_mir_build/src/build/expr/stmt.rs | 3 ++- .../rustc_mir_build/src/build/matches/mod.rs | 4 ++-- compiler/rustc_mir_build/src/build/mod.rs | 4 ++-- compiler/rustc_mir_build/src/build/scope.rs | 4 ++-- 8 files changed, 26 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/block.rs b/compiler/rustc_mir_build/src/build/block.rs index 813fd17606daf..c608e5c63d842 100644 --- a/compiler/rustc_mir_build/src/build/block.rs +++ b/compiler/rustc_mir_build/src/build/block.rs @@ -71,11 +71,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { StmtKind::Expr { scope, expr } => { this.block_context.push(BlockFrame::Statement { ignores_expr_result: true }); let si = (*scope, source_info); - unpack!( - block = this.in_scope(si, LintLevel::Inherited, |this| { + block = this + .in_scope(si, LintLevel::Inherited, |this| { this.stmt_expr(block, *expr, Some(*scope)) }) - ); + .into_block(); } StmtKind::Let { remainder_scope, @@ -166,14 +166,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let dummy_place = this.temp(this.tcx.types.never, else_block_span); let failure_entry = this.cfg.start_new_block(); let failure_block; - unpack!( - failure_block = this.ast_block( + failure_block = this + .ast_block( dummy_place, failure_entry, *else_block, this.source_info(else_block_span), ) - ); + .into_block(); this.cfg.terminate( failure_block, this.source_info(else_block_span), @@ -267,8 +267,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let initializer_span = this.thir[init].span; let scope = (*init_scope, source_info); - unpack!( - block = this.in_scope(scope, *lint_level, |this| { + block = this + .in_scope(scope, *lint_level, |this| { this.declare_bindings( visibility_scope, remainder_span, @@ -279,7 +279,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.expr_into_pattern(block, &pattern, init) // irrefutable pattern }) - ) + .into_block(); } else { let scope = (*init_scope, source_info); let _: BlockAnd<()> = this.in_scope(scope, *lint_level, |this| { @@ -333,7 +333,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.block_context .push(BlockFrame::TailExpr { tail_result_is_ignored, span: expr.span }); - unpack!(block = this.expr_into_dest(destination, block, expr_id)); + block = this.expr_into_dest(destination, block, expr_id).into_block(); let popped = this.block_context.pop(); assert!(popped.is_some_and(|bf| bf.is_tail_expr())); @@ -355,7 +355,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Finally, we pop all the let scopes before exiting out from the scope of block // itself. for scope in let_scope_stack.into_iter().rev() { - unpack!(block = this.pop_scope((*scope, source_info), block)); + block = this.pop_scope((*scope, source_info), block).into_block(); } // Restore the original source scope. this.source_scope = outer_source_scope; diff --git a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs index f0725e18b46e2..40cfe563accea 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -185,13 +185,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.cfg.push_assign(block, source_info, Place::from(result), box_); // initialize the box contents: - unpack!( - block = this.expr_into_dest( - this.tcx.mk_place_deref(Place::from(result)), - block, - value, - ) - ); + block = this + .expr_into_dest(this.tcx.mk_place_deref(Place::from(result)), block, value) + .into_block(); block.and(Rvalue::Use(Operand::Move(Place::from(result)))) } ExprKind::Cast { source } => { diff --git a/compiler/rustc_mir_build/src/build/expr/as_temp.rs b/compiler/rustc_mir_build/src/build/expr/as_temp.rs index 607c7c3259c18..82673582e796d 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_temp.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_temp.rs @@ -112,7 +112,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } - unpack!(block = this.expr_into_dest(temp_place, block, expr_id)); + block = this.expr_into_dest(temp_place, block, expr_id).into_block(); if let Some(temp_lifetime) = temp_lifetime { this.schedule_drop(expr_span, temp_lifetime, temp, DropKind::Value); diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index 0e5337ba4e12d..9cd958a21da4d 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -107,7 +107,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // If there is an `else` arm, lower it into `else_blk`. if let Some(else_expr) = else_opt { - unpack!(else_blk = this.expr_into_dest(destination, else_blk, else_expr)); + else_blk = this.expr_into_dest(destination, else_blk, else_expr).into_block(); } else { // There is no `else` arm, so we know both arms have type `()`. // Generate the implicit `else {}` by assigning unit. @@ -506,7 +506,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // These cases don't actually need a destination ExprKind::Assign { .. } | ExprKind::AssignOp { .. } => { - unpack!(block = this.stmt_expr(block, expr_id, None)); + block = this.stmt_expr(block, expr_id, None).into_block(); this.cfg.push_assign_unit(block, source_info, destination, this.tcx); block.unit() } @@ -515,7 +515,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | ExprKind::Break { .. } | ExprKind::Return { .. } | ExprKind::Become { .. } => { - unpack!(block = this.stmt_expr(block, expr_id, None)); + block = this.stmt_expr(block, expr_id, None).into_block(); // No assign, as these have type `!`. block.unit() } diff --git a/compiler/rustc_mir_build/src/build/expr/stmt.rs b/compiler/rustc_mir_build/src/build/expr/stmt.rs index 2bdeb579a02de..088d6631d959d 100644 --- a/compiler/rustc_mir_build/src/build/expr/stmt.rs +++ b/compiler/rustc_mir_build/src/build/expr/stmt.rs @@ -42,7 +42,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { if lhs_expr.ty.needs_drop(this.tcx, this.param_env) { let rhs = unpack!(block = this.as_local_rvalue(block, rhs)); let lhs = unpack!(block = this.as_place(block, lhs)); - unpack!(block = this.build_drop_and_replace(block, lhs_expr.span, lhs, rhs)); + block = + this.build_drop_and_replace(block, lhs_expr.span, lhs, rhs).into_block(); } else { let rhs = unpack!(block = this.as_local_rvalue(block, rhs)); let lhs = unpack!(block = this.as_place(block, lhs)); diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index be1ea0f8ba9a4..bffd325e31187 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -625,7 +625,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { OutsideGuard, ScheduleDrops::Yes, ); - unpack!(block = self.expr_into_dest(place, block, initializer_id)); + block = self.expr_into_dest(place, block, initializer_id).into_block(); // Inject a fake read, see comments on `FakeReadCause::ForLet`. let source_info = self.source_info(irrefutable_pat.span); @@ -664,7 +664,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { OutsideGuard, ScheduleDrops::Yes, ); - unpack!(block = self.expr_into_dest(place, block, initializer_id)); + block = self.expr_into_dest(place, block, initializer_id).into_block(); // Inject a fake read, see comments on `FakeReadCause::ForLet`. let pattern_source_info = self.source_info(irrefutable_pat.span); diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index 04aaa806d5946..2793a7d873621 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -584,7 +584,7 @@ fn construct_const<'a, 'tcx>( Builder::new(thir, infcx, def, hir_id, span, 0, const_ty, const_ty_span, None); let mut block = START_BLOCK; - unpack!(block = builder.expr_into_dest(Place::return_place(), block, expr)); + block = builder.expr_into_dest(Place::return_place(), block, expr).into_block(); let source_info = builder.source_info(span); builder.cfg.terminate(block, source_info, TerminatorKind::Return); @@ -966,7 +966,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { Some((Some(&place), span)), ); let place_builder = PlaceBuilder::from(local); - unpack!(block = self.place_into_pattern(block, pat, place_builder, false)); + block = self.place_into_pattern(block, pat, place_builder, false).into_block(); } } self.source_scope = original_source_scope; diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index d6be0e7a252e3..a2424b0fef127 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -587,7 +587,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.push_scope(region_scope); let mut block; let rv = unpack!(block = f(self)); - unpack!(block = self.pop_scope(region_scope, block)); + block = self.pop_scope(region_scope, block).into_block(); self.source_scope = source_scope; debug!(?block); block.and(rv) @@ -659,7 +659,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { (Some(destination), Some(value)) => { debug!("stmt_expr Break val block_context.push(SubExpr)"); self.block_context.push(BlockFrame::SubExpr); - unpack!(block = self.expr_into_dest(destination, block, value)); + block = self.expr_into_dest(destination, block, value).into_block(); self.block_context.pop(); } (Some(destination), None) => {