From 570376c49647d8565a639e9e190916fe2246c26f Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 1 Mar 2024 22:13:03 +1100 Subject: [PATCH] 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. --- .../rustc_mir_build/src/build/expr/into.rs | 2 -- .../rustc_mir_build/src/build/matches/mod.rs | 34 ++++--------------- compiler/rustc_mir_build/src/build/scope.rs | 21 +++++++----- 3 files changed, 19 insertions(+), 38 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index 69f3d3101fa21..8a9dc34bc7bad 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -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 )); @@ -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. diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 29ee07f5d79b7..31591983101f3 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -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, - /// 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. @@ -58,19 +55,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block: BasicBlock, expr_id: ExprId, temp_scope_override: Option, - 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 }, ) } @@ -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( @@ -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 } => { @@ -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, @@ -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() } @@ -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, span: Span, declare_bindings: bool, @@ -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); @@ -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 ) @@ -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) diff --git a/compiler/rustc_mir_build/src/build/scope.rs b/compiler/rustc_mir_build/src/build/scope.rs index 04740a9629152..961502566baaa 100644 --- a/compiler/rustc_mir_build/src/build/scope.rs +++ b/compiler/rustc_mir_build/src/build/scope.rs @@ -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;