Skip to content

Commit

Permalink
Replace a magic boolean with enum ScheduleDrops
Browse files Browse the repository at this point in the history
  • Loading branch information
Zalathar committed Jun 30, 2024
1 parent 3b22589 commit ed07712
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 18 deletions.
18 changes: 15 additions & 3 deletions compiler/rustc_mir_build/src/build/block.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
},
)
Expand Down
56 changes: 41 additions & 15 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ mod simplify;
mod test;
mod util;

use std::assert_matches::assert_matches;
use std::borrow::Borrow;
use std::mem;

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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);
},
Expand All @@ -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`.
Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -827,15 +849,15 @@ 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
&& matches!(schedule_drop, ScheduleDrops::Yes)
{
self.schedule_drop(span, region_scope, local_id, DropKind::Storage);
}
Expand Down Expand Up @@ -2112,7 +2134,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);
Expand Down Expand Up @@ -2323,10 +2345,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,
);
Expand Down Expand Up @@ -2376,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<Item = &'b Binding<'tcx>>,
) where
'tcx: 'b,
Expand Down Expand Up @@ -2429,7 +2455,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
Expand All @@ -2454,7 +2480,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
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 {
Expand Down

0 comments on commit ed07712

Please sign in to comment.