Replace a magic boolean with enum ScheduleDrops

rust-lang · Jun 26, 2024 · c0a0b5d · c0a0b5d
1 parent 3dccb9d
commit c0a0b5d
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 22 deletions.
diff --git a/compiler/rustc_mir_build/src/build/block.rs b/compiler/rustc_mir_build/src/build/block.rs
@@ -1,4 +1,4 @@
-use crate::build::matches::{DeclareLetBindings, EmitStorageLive};
+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;
@@ -202,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;
@@ -292,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/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;
 
@@ -74,6 +75,17 @@ pub(crate) enum EmitStorageLive {
     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> {
     /// Lowers a condition in a way that ensures that variables bound in any let
     /// expressions are definitely initialized in the if body.
@@ -535,7 +547,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 fake_borrow_temps,
                 scrutinee_span,
                 arm_match_scope,
-                true,
+                ScheduleDrops::Yes,
                 emit_storage_live,
             )
         } else {
@@ -554,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
@@ -576,7 +588,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                         emit_storage_live,
                     );
                     if arm.is_none() {
-                        schedule_drops = false;
+                        schedule_drops = ScheduleDrops::No;
                     }
                     self.cfg.goto(binding_end, outer_source_info, target_block);
                 },
@@ -602,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`.
@@ -636,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`.
@@ -827,17 +849,21 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         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);
         self.cfg.push(block, Statement { source_info, kind: StatementKind::StorageLive(local_id) });
         // 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
-        {
-            self.schedule_drop(span, region_scope, local_id, DropKind::Storage);
+        if let Some(region_scope) = self.region_scope_tree.var_scope(var.0.local_id) {
+            // Match exhaustively, so that it's easy to check how `ScheduleDrops` is used.
+            match schedule_drop {
+                ScheduleDrops::Yes => {
+                    self.schedule_drop(span, region_scope, local_id, DropKind::Storage);
+                }
+                ScheduleDrops::No => {}
+            }
         }
         Place::from(local_id)
     }
@@ -2112,7 +2138,7 @@ 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,
+        schedule_drops: ScheduleDrops,
         emit_storage_live: EmitStorageLive,
     ) -> BasicBlock {
         debug!("bind_and_guard_matched_candidate(candidate={:?})", candidate);
@@ -2323,10 +2349,14 @@ 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,
                 emit_storage_live,
             );
@@ -2376,7 +2406,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<Item = &'b Binding<'tcx>>,
     ) where
         'tcx: 'b,
@@ -2429,7 +2459,7 @@ 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<Item = &'b Binding<'tcx>>,
         emit_storage_live: EmitStorageLive,
     ) where
@@ -2454,8 +2484,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                     schedule_drops,
                 ),
             };
-            if schedule_drops {
-                self.schedule_drop_for_binding(binding.var_id, binding.span, OutsideGuard);
+            match schedule_drops {
+                ScheduleDrops::Yes => {
+                    self.schedule_drop_for_binding(binding.var_id, binding.span, OutsideGuard);
+                }
+                ScheduleDrops::No => {}
             }
             let rvalue = match binding.binding_mode.0 {
                 ByRef::No => Rvalue::Use(self.consume_by_copy_or_move(binding.source)),