diff --git a/compiler/rustc_mir_build/src/builder/block.rs b/compiler/rustc_mir_build/src/builder/block.rs index 7d85579325751..553b7af91e30c 100644 --- a/compiler/rustc_mir_build/src/builder/block.rs +++ b/compiler/rustc_mir_build/src/builder/block.rs @@ -166,18 +166,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // should never be used to take values at the end of the failure // block. let dummy_place = this.temp(this.tcx.types.never, else_block_span); - let failure_entry = this.cfg.start_new_block(); - let failure_block; - failure_block = this + // An unsuccessful match will jump to this block. + let failure_entry_block = this.cfg.start_new_block(); + let failure_end_block = this .ast_block( dummy_place, - failure_entry, + failure_entry_block, *else_block, this.source_info(else_block_span), ) .into_block(); this.cfg.terminate( - failure_block, + failure_end_block, this.source_info(else_block_span), TerminatorKind::Unreachable, ); @@ -193,7 +193,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let initializer_span = this.thir[*initializer].span; let scope = (*init_scope, source_info); let lint_level = LintLevel::Explicit(*hir_id); - let failure_and_block = this.in_scope(scope, lint_level, |this| { + + // Lower the initializer and test it against the pattern, leading to a + // true path (successful match) and a false path (failure). + let true_and_false_blocks = this.in_scope(scope, lint_level, |this| { this.declare_bindings( visibility_scope, remainder_span, @@ -202,8 +205,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { Some((Some(&destination), initializer_span)), ); let else_block_span = this.thir[*else_block].span; - let (matching, failure) = + let (true_block, false_block) = this.in_if_then_scope(last_remainder_scope, else_block_span, |this| { + // Bypass `lower_if_condition` and call `lower_let_expr` directly, + // since we don't have an actual THIR let-expression here. this.lower_let_expr( block, *initializer, @@ -213,10 +218,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { DeclareLetBindings::No, ) }); - matching.and(failure) + // Pack `(true_block, false_block)` into `BlockAnd`. + true_block.and(false_block) }); - let failure = unpack!(block = failure_and_block); - this.cfg.goto(failure, source_info, failure_entry); + // Unpack `BlockAnd` into `(true_block, false_block)`. + let (true_block, false_block); + false_block = unpack!(true_block = true_and_false_blocks); + + // Proceed along the successful path, or jump to the failure path. + block = true_block; + this.cfg.goto(false_block, source_info, failure_entry_block); if let Some(source_scope) = visibility_scope { this.source_scope = source_scope; diff --git a/compiler/rustc_mir_build/src/builder/coverageinfo.rs b/compiler/rustc_mir_build/src/builder/coverageinfo.rs index 2e29600c9339b..67135f2677a6e 100644 --- a/compiler/rustc_mir_build/src/builder/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/builder/coverageinfo.rs @@ -232,13 +232,13 @@ impl<'tcx> Builder<'_, 'tcx> { *block = join_block; } - /// If branch coverage is enabled, inject marker statements into `then_block` - /// and `else_block`, and record their IDs in the table of branch spans. + /// If branch coverage is enabled, inject marker statements into `true_block` + /// and `false_block`, and record their IDs in the table of branch spans. pub(crate) fn visit_coverage_branch_condition( &mut self, mut expr_id: ExprId, - mut then_block: BasicBlock, - mut else_block: BasicBlock, + mut true_block: BasicBlock, + mut false_block: BasicBlock, ) { // Bail out if coverage is not enabled for this function. let Some(coverage_info) = self.coverage_info.as_mut() else { return }; @@ -248,13 +248,13 @@ impl<'tcx> Builder<'_, 'tcx> { if let Some(&NotInfo { enclosing_not, is_flipped }) = coverage_info.nots.get(&expr_id) { expr_id = enclosing_not; if is_flipped { - std::mem::swap(&mut then_block, &mut else_block); + std::mem::swap(&mut true_block, &mut false_block); } } let source_info = SourceInfo { span: self.thir[expr_id].span, scope: self.source_scope }; - coverage_info.register_two_way_branch(&mut self.cfg, source_info, then_block, else_block); + coverage_info.register_two_way_branch(&mut self.cfg, source_info, true_block, false_block); } /// If branch coverage is enabled, inject marker statements into `true_block` diff --git a/compiler/rustc_mir_build/src/builder/expr/into.rs b/compiler/rustc_mir_build/src/builder/expr/into.rs index 13a64346c36c4..39b6389018c8e 100644 --- a/compiler/rustc_mir_build/src/builder/expr/into.rs +++ b/compiler/rustc_mir_build/src/builder/expr/into.rs @@ -14,7 +14,7 @@ use rustc_trait_selection::infer::InferCtxtExt; use tracing::{debug, instrument}; use crate::builder::expr::category::{Category, RvalueFunc}; -use crate::builder::matches::{DeclareLetBindings, Exhaustive, HasMatchGuard}; +use crate::builder::matches::{DeclareLetBindings, Exhaustive, HasMatchGuard, LowerIfCondArgs}; use crate::builder::scope::LintLevel; use crate::builder::{BlockAnd, BlockAndExtension, BlockFrame, Builder, NeedsTemporary}; use crate::diagnostics::{LoopMatchArmWithGuard, LoopMatchUnsupportedType}; @@ -67,7 +67,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let then_source_info = this.source_info(then_span); let condition_scope = this.local_scope(); - let then_and_else_blocks = this.in_scope( + let true_and_false_blocks = this.in_scope( (if_then_scope, then_source_info), LintLevel::Inherited, |this| { @@ -81,47 +81,50 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.source_info(then_span) }; - // Lower the condition, and have it branch into `then` and `else` blocks. - let (then_block, else_block) = + // Lower the condition, and have it branch into *true* and *false* blocks. + let (true_block, false_block) = this.in_if_then_scope(condition_scope, then_span, |this| { - let then_blk = this - .then_else_break( + let true_block = this + .lower_if_condition( block, cond, - Some(condition_scope), // Temp scope - source_info, - DeclareLetBindings::Yes, // Declare `let` bindings normally + LowerIfCondArgs { + temp_scope_override: Some(condition_scope), + variable_source_info: source_info, + declare_let_bindings: DeclareLetBindings::Yes, + }, ) .into_block(); // Lower the `then` arm into its block. - this.expr_into_dest(destination, then_blk, then) + this.expr_into_dest(destination, true_block, then) }); - // Pack `(then_block, else_block)` into `BlockAnd`. - then_block.and(else_block) + // Pack `(true_block, false_block)` into `BlockAnd`. + true_block.and(false_block) }, ); - // Unpack `BlockAnd` into `(then_blk, else_blk)`. - let (then_blk, mut else_blk); - else_blk = unpack!(then_blk = then_and_else_blocks); + // Unpack `BlockAnd` into `(true_block, false_block)`. + let (true_block, mut false_block); + false_block = unpack!(true_block = true_and_false_blocks); - // If there is an `else` arm, lower it into `else_blk`. + // If there is an `else` arm, lower it into `false_block`. if let Some(else_expr) = else_opt { - else_blk = this.expr_into_dest(destination, else_blk, else_expr).into_block(); + false_block = + this.expr_into_dest(destination, false_block, else_expr).into_block(); } else { // There is no `else` arm, so we know both arms have type `()`. // Generate the implicit `else {}` by assigning unit. let correct_si = this.source_info(expr_span.shrink_to_hi()); - this.cfg.push_assign_unit(else_blk, correct_si, destination, this.tcx); + this.cfg.push_assign_unit(false_block, correct_si, destination, this.tcx); } // The `then` and `else` arms have been lowered into their respective // blocks, so make both of them meet up in a new block. let join_block = this.cfg.start_new_block(); - this.cfg.goto(then_blk, source_info, join_block); - this.cfg.goto(else_blk, source_info, join_block); + this.cfg.goto(true_block, source_info, join_block); + this.cfg.goto(false_block, source_info, join_block); join_block.unit() } ExprKind::Let { .. } => { @@ -158,48 +161,50 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let source_info = this.source_info(expr.span); // We first evaluate the left-hand side of the predicate ... - let (then_block, else_block) = + let (true_block, false_block) = this.in_if_then_scope(condition_scope, expr.span, |this| { - this.then_else_break( + this.lower_if_condition( block, lhs, - Some(condition_scope), // Temp scope - source_info, - // This flag controls how inner `let` expressions are lowered, - // but either way there shouldn't be any of those in here. - DeclareLetBindings::LetNotPermitted, + LowerIfCondArgs { + temp_scope_override: Some(condition_scope), + variable_source_info: source_info, + declare_let_bindings: DeclareLetBindings::LetNotPermitted, + }, ) }); - let (short_circuit, continuation, constant) = match op { - LogicalOp::And => (else_block, then_block, false), - LogicalOp::Or => (then_block, else_block, true), - }; + // At this point, the control flow splits into a short-circuiting path // and a continuation path. // - If the operator is `&&`, passing `lhs` leads to continuation of evaluation on `rhs`; // failing it leads to the short-circuting path which assigns `false` to the place. // - If the operator is `||`, failing `lhs` leads to continuation of evaluation on `rhs`; // passing it leads to the short-circuting path which assigns `true` to the place. + let (short_circuit_block, short_circuit_value, continue_block) = match op { + LogicalOp::And => (false_block, false, true_block), + LogicalOp::Or => (true_block, true, false_block), + }; this.cfg.push_assign_constant( - short_circuit, + short_circuit_block, source_info, destination, ConstOperand { span: expr.span, user_ty: None, - const_: Const::from_bool(this.tcx, constant), + const_: Const::from_bool(this.tcx, short_circuit_value), }, ); let mut rhs_block = - this.expr_into_dest(destination, continuation, rhs).into_block(); + this.expr_into_dest(destination, continue_block, 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); - let target = this.cfg.start_new_block(); - this.cfg.goto(rhs_block, source_info, target); - this.cfg.goto(short_circuit, source_info, target); - target.unit() + // Reunite the continuation path and the short-circuit path. + let join_block = this.cfg.start_new_block(); + this.cfg.goto(rhs_block, source_info, join_block); + this.cfg.goto(short_circuit_block, source_info, join_block); + join_block.unit() } ExprKind::Loop { body } => { // [block] diff --git a/compiler/rustc_mir_build/src/builder/matches/mod.rs b/compiler/rustc_mir_build/src/builder/matches/mod.rs index ddeb9e084b21d..2085213326188 100644 --- a/compiler/rustc_mir_build/src/builder/matches/mod.rs +++ b/compiler/rustc_mir_build/src/builder/matches/mod.rs @@ -40,18 +40,26 @@ mod test; mod user_ty; mod util; -/// Arguments to [`Builder::then_else_break_inner`] that are usually forwarded +/// Arguments to [`Builder::lower_if_condition`] that are usually forwarded /// to recursive invocations. #[derive(Clone, Copy)] -struct ThenElseArgs { +pub(crate) struct LowerIfCondArgs { /// Used as the temp scope for lowering `expr`. If absent (for match guards), /// `self.local_scope()` is used. - temp_scope_override: Option, - variable_source_info: SourceInfo, + pub(crate) temp_scope_override: Option, + pub(crate) variable_source_info: SourceInfo, /// Determines how bindings should be handled when lowering `let` expressions. /// /// Forwarded to [`Builder::lower_let_expr`] when lowering [`ExprKind::Let`]. - declare_let_bindings: DeclareLetBindings, + pub(crate) declare_let_bindings: DeclareLetBindings, +} + +impl LowerIfCondArgs { + /// Returns a copy of `self` with [`DeclareLetBindings::LetNotPermitted`]. + /// Used when recursing into a sub-condition that does not permit `let` (e.g. `||` or `!`). + fn let_not_permitted(self) -> Self { + LowerIfCondArgs { declare_let_bindings: DeclareLetBindings::LetNotPermitted, ..self } + } } /// Should lowering a `let` expression also declare its bindings? @@ -83,32 +91,19 @@ pub(crate) enum ScheduleDrops { } impl<'a, 'tcx> Builder<'a, 'tcx> { - /// Lowers a condition in a way that ensures that variables bound in any let - /// expressions are definitely initialized in the if body. + /// Lowers the condition for an `if`-expression or similar construct + /// (including `&&` and `||` expressions, and match-guard conditions). /// - /// If `declare_let_bindings` is false then variables created in `let` - /// expressions will not be declared. This is for if let guards on arms with - /// an or pattern, where the guard is lowered multiple times. - pub(crate) fn then_else_break( - &mut self, - block: BasicBlock, - expr_id: ExprId, - temp_scope_override: Option, - variable_source_info: SourceInfo, - declare_let_bindings: DeclareLetBindings, - ) -> BlockAnd<()> { - self.then_else_break_inner( - block, - expr_id, - ThenElseArgs { temp_scope_override, variable_source_info, declare_let_bindings }, - ) - } - - fn then_else_break_inner( + /// Must be called within [`Builder::in_if_then_scope`], which keeps track + /// of drop scope and knows where to break to if the condition is false. + /// + /// Returns the block for the *true* arm of the condition check. + /// The *true* and *false* arms are returned by [`Builder::in_if_then_scope`]. + pub(crate) fn lower_if_condition( &mut self, block: BasicBlock, // Block that the condition and branch will be lowered into expr_id: ExprId, // Condition expression to lower - args: ThenElseArgs, + args: LowerIfCondArgs, ) -> BlockAnd<()> { let this = self; // See "LET_THIS_SELF". let expr = &this.thir[expr_id]; @@ -116,44 +111,40 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { match expr.kind { ExprKind::LogicalOp { op: LogicalOp::And, lhs, rhs } => { - 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() + // A condition of `lhs && rhs` is fairly straightforward. + // We can just lower them in sequence, and break if either is false. + let lhs_true_block = this.lower_if_condition(block, lhs, args).into_block(); + let rhs_true_block = + this.lower_if_condition(lhs_true_block, rhs, args).into_block(); + rhs_true_block.unit() } ExprKind::LogicalOp { op: LogicalOp::Or, lhs, rhs } => { + // A condition of `lhs || rhs` is more complicated, because we need to + // short-circuit if `lhs` is *true*. So an inner condition-scope is needed. + // See . let local_scope = this.local_scope(); - let (lhs_success_block, failure_block) = + let (lhs_true_block, lhs_false_block) = this.in_if_then_scope(local_scope, expr_span, |this| { - this.then_else_break_inner( - block, - lhs, - ThenElseArgs { - declare_let_bindings: DeclareLetBindings::LetNotPermitted, - ..args - }, - ) + this.lower_if_condition(block, lhs, args.let_not_permitted()) }); - let rhs_success_block = this - .then_else_break_inner( - failure_block, - rhs, - ThenElseArgs { - declare_let_bindings: DeclareLetBindings::LetNotPermitted, - ..args - }, - ) + let rhs_true_block = this + .lower_if_condition(lhs_false_block, rhs, args.let_not_permitted()) .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` + // Make the LHS-true and RHS-true arms converge to a common block. + // (We can't just make LHS goto RHS, because `rhs_true_block` // might contain statements that we don't want on the LHS path.) let success_block = this.cfg.start_new_block(); - this.cfg.goto(lhs_success_block, args.variable_source_info, success_block); - this.cfg.goto(rhs_success_block, args.variable_source_info, success_block); + this.cfg.goto(lhs_true_block, args.variable_source_info, success_block); + this.cfg.goto(rhs_true_block, args.variable_source_info, success_block); success_block.unit() } ExprKind::Unary { op: UnOp::Not, arg } => { + // For a condition of `!cond`, lower `cond` as its own condition, + // then invert the meaning of the true/false blocks. + // This avoids an intermediate temporary for negating the condition value. + // See . + // Improve branch coverage instrumentation by noting conditions // nested within one or more `!` expressions. // (Skipped if branch coverage is not enabled.) @@ -162,32 +153,26 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } let local_scope = this.local_scope(); - let (success_block, failure_block) = + let (true_block, false_block) = this.in_if_then_scope(local_scope, expr_span, |this| { // Help out coverage instrumentation by injecting a dummy statement with // the original condition's span (including `!`). This fixes #115468. if this.tcx.sess.instrument_coverage() { this.cfg.push_coverage_span_marker(block, this.source_info(expr_span)); } - this.then_else_break_inner( - block, - arg, - ThenElseArgs { - declare_let_bindings: DeclareLetBindings::LetNotPermitted, - ..args - }, - ) + this.lower_if_condition(block, arg, args.let_not_permitted()) }); - this.break_for_else(success_block, args.variable_source_info); - failure_block.unit() + // Break if the condition was true; proceed if the condition was false. + this.break_from_if_then_scope(true_block, args.variable_source_info); + false_block.unit() } ExprKind::Scope { region_scope, hir_id, value } => { let region_scope = (region_scope, this.source_info(expr_span)); this.in_scope(region_scope, LintLevel::Explicit(hir_id), |this| { - this.then_else_break_inner(block, value, args) + this.lower_if_condition(block, value, args) }) } - ExprKind::Use { source } => this.then_else_break_inner(block, source, args), + ExprKind::Use { source } => this.lower_if_condition(block, source, args), ExprKind::Let { expr, ref pat } => this.lower_let_expr( block, expr, @@ -196,7 +181,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { args.variable_source_info.span, args.declare_let_bindings, ), + _ => { + // The condition is an ordinary boolean-valued expression, + // so lower it normally and branch on the result. let mut block = block; let temp_scope = args.temp_scope_override.unwrap_or_else(|| this.local_scope()); let mutability = Mutability::Mut; @@ -215,19 +203,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let operand = Operand::Move(Place::from(place)); - let then_block = this.cfg.start_new_block(); - let else_block = this.cfg.start_new_block(); - let term = TerminatorKind::if_(operand, then_block, else_block); + let true_block = this.cfg.start_new_block(); + let false_block = this.cfg.start_new_block(); + let term = TerminatorKind::if_(operand, true_block, false_block); // Record branch coverage info for this condition. // (Does nothing if branch coverage is not enabled.) - this.visit_coverage_branch_condition(expr_id, then_block, else_block); + this.visit_coverage_branch_condition(expr_id, true_block, false_block); let source_info = this.source_info(expr_span); this.cfg.terminate(block, source_info, term); - this.break_for_else(else_block, source_info); + this.break_from_if_then_scope(false_block, source_info); - then_block.unit() + true_block.unit() } } } @@ -2336,6 +2324,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// /// Use [`DeclareLetBindings`] to control whether the `let` bindings are /// declared or not. + /// + /// Must be called within a [`Builder::in_if_then_scope`], to indicate where + /// to break to if the `let` fails to match. pub(crate) fn lower_let_expr( &mut self, mut block: BasicBlock, @@ -2357,7 +2348,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); let [branch] = built_tree.branches.try_into().unwrap(); - self.break_for_else(built_tree.otherwise_block, self.source_info(expr_span)); + // If pattern-matching failed, break out of the enclosing if-then scope. + self.break_from_if_then_scope(built_tree.otherwise_block, self.source_info(expr_span)); match declare_let_bindings { DeclareLetBindings::Yes => { @@ -2445,15 +2437,18 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let mut guard_span = rustc_span::DUMMY_SP; - let (post_guard_block, otherwise_post_guard_block) = + let (guard_true_block, guard_false_block) = self.in_if_then_scope(match_scope, guard_span, |this| { guard_span = this.thir[guard].span; - this.then_else_break( + this.lower_if_condition( block, guard, - None, // Use `self.local_scope()` as the temp scope - this.source_info(arm.span), - DeclareLetBindings::No, // For guards, `let` bindings are declared separately + LowerIfCondArgs { + temp_scope_override: None, // Use `this.local_scope()`. + variable_source_info: this.source_info(arm.span), + // For guards, `let` bindings are declared separately. + declare_let_bindings: DeclareLetBindings::No, + }, ) }); @@ -2471,10 +2466,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { for &(_, temp, _) in fake_borrows { let cause = FakeReadCause::ForMatchGuard; - self.cfg.push_fake_read(post_guard_block, guard_end, cause, Place::from(temp)); + self.cfg.push_fake_read(guard_true_block, guard_end, cause, Place::from(temp)); } - self.cfg.goto(otherwise_post_guard_block, source_info, sub_branch.otherwise_block); + self.cfg.goto(guard_false_block, source_info, sub_branch.otherwise_block); // We want to ensure that the matched candidates are bound // after we have confirmed this candidate *and* any @@ -2511,16 +2506,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { for binding in by_value_bindings.clone() { let local_id = self.var_local_id(binding.var_id, RefWithinGuard); let cause = FakeReadCause::ForGuardBinding; - self.cfg.push_fake_read(post_guard_block, guard_end, cause, Place::from(local_id)); + self.cfg.push_fake_read(guard_true_block, guard_end, cause, Place::from(local_id)); } // Only schedule drops for the last sub-branch we lower. self.bind_matched_candidate_for_arm_body( - post_guard_block, + guard_true_block, schedule_drops, by_value_bindings, ); - post_guard_block + guard_true_block } else { // (Here, it is not too early to bind the matched // candidate on `block`, because there is no guard result diff --git a/compiler/rustc_mir_build/src/builder/scope.rs b/compiler/rustc_mir_build/src/builder/scope.rs index 26e89cedb3070..b7aaa0a52816a 100644 --- a/compiler/rustc_mir_build/src/builder/scope.rs +++ b/compiler/rustc_mir_build/src/builder/scope.rs @@ -638,9 +638,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// guards. /// /// For an if-let chain: - /// - /// if let Some(x) = a && let Some(y) = b && let Some(z) = c { ... } - /// + /// ```rust,ignore(illustrative) + /// if let Some(x) = a && let Some(y) = b && let Some(z) = c { ... } + /// ``` /// There are three possible ways the condition can be false and we may have /// to drop `x`, `x` and `y`, or neither depending on which binding fails. /// To handle this correctly we use a `DropTree` in a similar way to a @@ -650,30 +650,31 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// - We don't need to keep a stack of scopes in the `Builder` because the /// 'else' paths will only leave the innermost scope. /// - This is also used for match guards. - pub(crate) fn in_if_then_scope( + /// + /// Returns blocks for the two condition outcomes, `(true_block, false_block)`. + pub(crate) fn in_if_then_scope( &mut self, region_scope: region::Scope, span: Span, - f: F, - ) -> (BasicBlock, BasicBlock) - where - F: FnOnce(&mut Builder<'a, 'tcx>) -> BlockAnd<()>, - { + // Closure that will lower the condition(s), register breaks, and return `true_block`. + f: impl FnOnce(&mut Builder<'a, 'tcx>) -> BlockAnd<()>, + ) -> (BasicBlock, BasicBlock) { 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 = f(self).into_block(); + let true_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| else_block_and.into_block(), - ); + // Lower any break paths (where the condition was false) + // into a drop tree that ends in `false_block`. + let false_block = self + .build_exit_tree(if_then_scope.else_drops, region_scope, span, None) + .map(|false_block: BlockAnd<()>| false_block.into_block()) + .unwrap_or_else(|| self.cfg.start_new_block()); - (then_block, else_block) + (true_block, false_block) } /// Convenience wrapper that pushes a scope and then executes `f` @@ -1054,12 +1055,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { return self.cfg.start_new_block().unit(); } - /// Sets up the drops for breaking from `block` due to an `if` condition - /// that turned out to be false. + /// Breaks out of the enclosing [`Builder::in_if_then_scope`] due to a + /// condition being false. + /// + /// This adds relevant drops in the drop tree, and adds a dummy terminator + /// that will become a real `goto` when the scope's drop tree is built. /// /// 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) { + pub(crate) fn break_from_if_then_scope(&mut self, block: BasicBlock, source_info: SourceInfo) { let if_then_scope = self .scopes .if_then_scope @@ -1969,7 +1973,7 @@ impl<'a, 'tcx: 'a> Builder<'a, 'tcx> { /// Build a drop tree for a breakable scope. /// /// If `continue_block` is `Some`, then the tree is for `continue` inside a - /// loop. Otherwise this is for `break` or `return`. + /// loop. Otherwise this is for `break`, `return`, or `if`. fn build_exit_tree( &mut self, mut drops: DropTree, @@ -2118,7 +2122,7 @@ impl<'tcx> DropTreeBuilder<'tcx> for ExitScopes { fn link_entry_point(cfg: &mut CFG<'tcx>, from: BasicBlock, to: BasicBlock) { // There should be an existing terminator with real source info and a // dummy TerminatorKind. Replace it with a proper goto. - // (The dummy is added by `break_scope` and `break_for_else`.) + // (The dummy is added by `break_scope` and `break_from_if_then_scope`.) let term = cfg.block_data_mut(from).terminator_mut(); if let TerminatorKind::UnwindResume = term.kind { term.kind = TerminatorKind::Goto { target: to };