diff --git a/compiler/rustc_mir_build/src/build/block.rs b/compiler/rustc_mir_build/src/build/block.rs index 476969a1bd7ad..5ccbd7c59cfba 100644 --- a/compiler/rustc_mir_build/src/build/block.rs +++ b/compiler/rustc_mir_build/src/build/block.rs @@ -1,3 +1,4 @@ +use crate::build::matches::{DeclareLetBindings, EmitStorageLive, ScheduleDrops}; use crate::build::ForGuard::OutsideGuard; use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder}; use rustc_middle::middle::region::Scope; @@ -201,7 +202,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { pattern, UserTypeProjections::none(), &mut |this, _, _, node, span, _, _| { - this.storage_live_binding(block, node, span, OutsideGuard, true); + this.storage_live_binding( + block, + node, + span, + OutsideGuard, + ScheduleDrops::Yes, + ); }, ); let else_block_span = this.thir[*else_block].span; @@ -213,8 +220,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { pattern, None, initializer_span, - false, - true, + DeclareLetBindings::No, + EmitStorageLive::No, ) }); matching.and(failure) @@ -291,7 +298,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { pattern, UserTypeProjections::none(), &mut |this, _, _, node, span, _, _| { - this.storage_live_binding(block, node, span, OutsideGuard, true); + this.storage_live_binding( + block, + node, + span, + OutsideGuard, + ScheduleDrops::Yes, + ); this.schedule_drop_for_binding(node, span, OutsideGuard); }, ) diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index 76bdc26a501a6..942c69b5c0a75 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -1,6 +1,7 @@ //! See docs in build/expr/mod.rs use crate::build::expr::category::{Category, RvalueFunc}; +use crate::build::matches::DeclareLetBindings; use crate::build::{BlockAnd, BlockAndExtension, BlockFrame, Builder, NeedsTemporary}; use rustc_ast::InlineAsmOptions; use rustc_data_structures::fx::FxHashMap; @@ -86,7 +87,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { cond, Some(condition_scope), // Temp scope source_info, - true, // Declare `let` bindings normally + DeclareLetBindings::Yes, // Declare `let` bindings normally )); // Lower the `then` arm into its block. @@ -163,7 +164,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { source_info, // This flag controls how inner `let` expressions are lowered, // but either way there shouldn't be any of those in here. - true, + DeclareLetBindings::LetNotPermitted, ) }); let (short_circuit, continuation, constant) = match op { diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 6d52c30823769..efed52231e3fa 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -28,6 +28,7 @@ mod simplify; mod test; mod util; +use std::assert_matches::assert_matches; use std::borrow::Borrow; use std::mem; @@ -39,9 +40,50 @@ struct ThenElseArgs { /// `self.local_scope()` is used. temp_scope_override: Option, variable_source_info: SourceInfo, + /// Determines how bindings should be handled when lowering `let` expressions. + /// /// Forwarded to [`Builder::lower_let_expr`] when lowering [`ExprKind::Let`]. - /// When false (for match guards), `let` bindings won't be declared. - declare_let_bindings: bool, + declare_let_bindings: DeclareLetBindings, +} + +/// Should lowering a `let` expression also declare its bindings? +/// +/// Used by [`Builder::lower_let_expr`] when lowering [`ExprKind::Let`]. +#[derive(Clone, Copy)] +pub(crate) enum DeclareLetBindings { + /// Yes, declare `let` bindings as normal for `if` conditions. + Yes, + /// No, don't declare `let` bindings, because the caller declares them + /// separately due to special requirements. + /// + /// Used for match guards and let-else. + No, + /// Let expressions are not permitted in this context, so it is a bug to + /// try to lower one (e.g inside lazy-boolean-or or boolean-not). + LetNotPermitted, +} + +/// Used by [`Builder::bind_matched_candidate_for_arm_body`] to determine +/// whether or not to call [`Builder::storage_live_binding`] to emit +/// [`StatementKind::StorageLive`]. +#[derive(Clone, Copy)] +pub(crate) enum EmitStorageLive { + /// Yes, emit `StorageLive` as normal. + Yes, + /// No, don't emit `StorageLive`. The caller has taken responsibility for + /// emitting `StorageLive` as appropriate. + No, +} + +/// Used by [`Builder::storage_live_binding`] and [`Builder::bind_matched_candidate_for_arm_body`] +/// to decide whether to schedule drops. +#[derive(Clone, Copy, Debug)] +pub(crate) enum ScheduleDrops { + /// Yes, the relevant functions should also schedule drops as appropriate. + Yes, + /// No, don't schedule drops. The caller has taken responsibility for any + /// appropriate drops. + No, } impl<'a, 'tcx> Builder<'a, 'tcx> { @@ -57,7 +99,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { expr_id: ExprId, temp_scope_override: Option, variable_source_info: SourceInfo, - declare_let_bindings: bool, + declare_let_bindings: DeclareLetBindings, ) -> BlockAnd<()> { self.then_else_break_inner( block, @@ -91,13 +133,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.then_else_break_inner( block, lhs, - ThenElseArgs { declare_let_bindings: true, ..args }, + ThenElseArgs { + declare_let_bindings: DeclareLetBindings::LetNotPermitted, + ..args + }, ) }); let rhs_success_block = unpack!(this.then_else_break_inner( failure_block, rhs, - ThenElseArgs { declare_let_bindings: true, ..args }, + ThenElseArgs { + declare_let_bindings: DeclareLetBindings::LetNotPermitted, + ..args + }, )); // Make the LHS and RHS success arms converge to a common block. @@ -127,7 +175,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.then_else_break_inner( block, arg, - ThenElseArgs { declare_let_bindings: true, ..args }, + ThenElseArgs { + declare_let_bindings: DeclareLetBindings::LetNotPermitted, + ..args + }, ) }); this.break_for_else(success_block, args.variable_source_info); @@ -147,7 +198,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { Some(args.variable_source_info.scope), args.variable_source_info.span, args.declare_let_bindings, - false, + EmitStorageLive::Yes, ), _ => { let mut block = block; @@ -440,7 +491,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { &fake_borrow_temps, scrutinee_span, Some((arm, match_scope)), - false, + EmitStorageLive::Yes, ); this.fixed_temps_scope = old_dedup_scope; @@ -485,7 +536,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fake_borrow_temps: &[(Place<'tcx>, Local, FakeBorrowKind)], scrutinee_span: Span, arm_match_scope: Option<(&Arm<'tcx>, region::Scope)>, - storages_alive: bool, + emit_storage_live: EmitStorageLive, ) -> BasicBlock { if candidate.subcandidates.is_empty() { // Avoid generating another `BasicBlock` when we only have one @@ -496,8 +547,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fake_borrow_temps, scrutinee_span, arm_match_scope, - true, - storages_alive, + ScheduleDrops::Yes, + emit_storage_live, ) } else { // It's helpful to avoid scheduling drops multiple times to save @@ -515,7 +566,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // To handle this we instead unschedule it's drop after each time // we lower the guard. let target_block = self.cfg.start_new_block(); - let mut schedule_drops = true; + let mut schedule_drops = ScheduleDrops::Yes; let arm = arm_match_scope.unzip().0; // We keep a stack of all of the bindings and type ascriptions // from the parent candidates that we visit, that also need to @@ -534,10 +585,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { scrutinee_span, arm_match_scope, schedule_drops, - storages_alive, + emit_storage_live, ); if arm.is_none() { - schedule_drops = false; + schedule_drops = ScheduleDrops::No; } self.cfg.goto(binding_end, outer_source_info, target_block); }, @@ -563,8 +614,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { match irrefutable_pat.kind { // Optimize the case of `let x = ...` to write directly into `x` PatKind::Binding { mode: BindingMode(ByRef::No, _), var, subpattern: None, .. } => { - let place = - self.storage_live_binding(block, var, irrefutable_pat.span, OutsideGuard, true); + let place = self.storage_live_binding( + block, + var, + irrefutable_pat.span, + OutsideGuard, + ScheduleDrops::Yes, + ); unpack!(block = self.expr_into_dest(place, block, initializer_id)); // Inject a fake read, see comments on `FakeReadCause::ForLet`. @@ -597,8 +653,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }, ascription: thir::Ascription { ref annotation, variance: _ }, } => { - let place = - self.storage_live_binding(block, var, irrefutable_pat.span, OutsideGuard, true); + let place = self.storage_live_binding( + block, + var, + irrefutable_pat.span, + OutsideGuard, + ScheduleDrops::Yes, + ); unpack!(block = self.expr_into_dest(place, block, initializer_id)); // Inject a fake read, see comments on `FakeReadCause::ForLet`. @@ -704,7 +765,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { &[], irrefutable_pat.span, None, - false, + EmitStorageLive::Yes, ) .unit() } @@ -780,13 +841,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } + /// Emits a [`StatementKind::StorageLive`] for the given var, and also + /// schedules a drop if requested (and possible). pub(crate) fn storage_live_binding( &mut self, block: BasicBlock, var: LocalVarId, span: Span, for_guard: ForGuard, - schedule_drop: bool, + schedule_drop: ScheduleDrops, ) -> Place<'tcx> { let local_id = self.var_local_id(var, for_guard); let source_info = self.source_info(span); @@ -794,7 +857,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Although there is almost always scope for given variable in corner cases // like #92893 we might get variable with no scope. if let Some(region_scope) = self.region_scope_tree.var_scope(var.0.local_id) - && schedule_drop + && matches!(schedule_drop, ScheduleDrops::Yes) { self.schedule_drop(span, region_scope, local_id, DropKind::Storage); } @@ -1991,8 +2054,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Pat binding - used for `let` and function parameters as well. impl<'a, 'tcx> Builder<'a, 'tcx> { - /// If the bindings have already been declared, set `declare_bindings` to - /// `false` to avoid duplicated bindings declaration; used for if-let guards. + /// Lowers a `let` expression that appears in a suitable context + /// (e.g. an `if` condition or match guard). + /// + /// Also used for lowering let-else statements, since they have similar + /// needs despite not actually using `let` expressions. + /// + /// Use [`DeclareLetBindings`] to control whether the `let` bindings are + /// declared or not. pub(crate) fn lower_let_expr( &mut self, mut block: BasicBlock, @@ -2000,8 +2069,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { pat: &Pat<'tcx>, source_scope: Option, scope_span: Span, - declare_bindings: bool, - storages_alive: bool, + declare_let_bindings: DeclareLetBindings, + emit_storage_live: EmitStorageLive, ) -> BlockAnd<()> { let expr_span = self.thir[expr_id].span; let scrutinee = unpack!(block = self.lower_scrutinee(block, expr_id, expr_span)); @@ -2017,10 +2086,22 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.break_for_else(otherwise_block, self.source_info(expr_span)); - if declare_bindings { - let expr_place = scrutinee.try_to_place(self); - let opt_expr_place = expr_place.as_ref().map(|place| (Some(place), expr_span)); - self.declare_bindings(source_scope, pat.span.to(scope_span), pat, None, opt_expr_place); + match declare_let_bindings { + DeclareLetBindings::Yes => { + let expr_place = scrutinee.try_to_place(self); + let opt_expr_place = expr_place.as_ref().map(|place| (Some(place), expr_span)); + self.declare_bindings( + source_scope, + pat.span.to(scope_span), + pat, + None, + opt_expr_place, + ); + } + DeclareLetBindings::No => {} // Caller is responsible for bindings. + DeclareLetBindings::LetNotPermitted => { + self.tcx.dcx().span_bug(expr_span, "let expression not expected in this context") + } } let success = self.bind_pattern( @@ -2029,7 +2110,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { &[], expr_span, None, - storages_alive, + emit_storage_live, ); // If branch coverage is enabled, record this branch. @@ -2053,8 +2134,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fake_borrows: &[(Place<'tcx>, Local, FakeBorrowKind)], scrutinee_span: Span, arm_match_scope: Option<(&Arm<'tcx>, region::Scope)>, - schedule_drops: bool, - storages_alive: bool, + schedule_drops: ScheduleDrops, + emit_storage_live: EmitStorageLive, ) -> BasicBlock { debug!("bind_and_guard_matched_candidate(candidate={:?})", candidate); @@ -2203,7 +2284,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { guard, None, // Use `self.local_scope()` as the temp scope this.source_info(arm.span), - false, // For guards, `let` bindings are declared separately + DeclareLetBindings::No, // For guards, `let` bindings are declared separately ) }); @@ -2264,12 +2345,16 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let cause = FakeReadCause::ForGuardBinding; self.cfg.push_fake_read(post_guard_block, guard_end, cause, Place::from(local_id)); } - assert!(schedule_drops, "patterns with guards must schedule drops"); + assert_matches!( + schedule_drops, + ScheduleDrops::Yes, + "patterns with guards must schedule drops" + ); self.bind_matched_candidate_for_arm_body( post_guard_block, - true, + ScheduleDrops::Yes, by_value_bindings, - storages_alive, + emit_storage_live, ); post_guard_block @@ -2281,7 +2366,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block, schedule_drops, bindings, - storages_alive, + emit_storage_live, ); block } @@ -2317,7 +2402,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fn bind_matched_candidate_for_guard<'b>( &mut self, block: BasicBlock, - schedule_drops: bool, + schedule_drops: ScheduleDrops, bindings: impl IntoIterator>, ) where 'tcx: 'b, @@ -2370,9 +2455,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fn bind_matched_candidate_for_arm_body<'b>( &mut self, block: BasicBlock, - schedule_drops: bool, + schedule_drops: ScheduleDrops, bindings: impl IntoIterator>, - storages_alive: bool, + emit_storage_live: EmitStorageLive, ) where 'tcx: 'b, { @@ -2382,21 +2467,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Assign each of the bindings. This may trigger moves out of the candidate. for binding in bindings { let source_info = self.source_info(binding.span); - let local = if storages_alive { + let local = match emit_storage_live { // Here storages are already alive, probably because this is a binding // from let-else. // We just need to schedule drop for the value. - self.var_local_id(binding.var_id, OutsideGuard).into() - } else { - self.storage_live_binding( + EmitStorageLive::No => self.var_local_id(binding.var_id, OutsideGuard).into(), + EmitStorageLive::Yes => self.storage_live_binding( block, binding.var_id, binding.span, OutsideGuard, schedule_drops, - ) + ), }; - if schedule_drops { + if matches!(schedule_drops, ScheduleDrops::Yes) { self.schedule_drop_for_binding(binding.var_id, binding.span, OutsideGuard); } let rvalue = match binding.binding_mode.0 {