Skip to content

Commit

Permalink
Start blocks early instead of making it complicated
Browse files Browse the repository at this point in the history
  • Loading branch information
@Nadrieril
Nadrieril committed Feb 12, 2024
1 parent f4cfd87 commit 61edca9
Show file tree
Hide file tree
Showing 66 changed files with 744 additions and 630 deletions.
111 changes: 51 additions & 60 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -319,54 +319,52 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// them.
let mut fake_borrows = match_has_guard.then(FxIndexSet::default);

let mut otherwise = None;
let otherwise_block = self.cfg.start_new_block();

// This will generate code to test scrutinee_place and
// branch to the appropriate arm block
self.match_candidates(
match_start_span,
scrutinee_span,
block,
&mut otherwise,
otherwise_block,
candidates,
&mut fake_borrows,
);

if let Some(otherwise_block) = otherwise {
// See the doc comment on `match_candidates` for why we may have an
// otherwise block. Match checking will ensure this is actually
// unreachable.
let source_info = self.source_info(scrutinee_span);

// Matching on a `scrutinee_place` with an uninhabited type doesn't
// generate any memory reads by itself, and so if the place "expression"
// contains unsafe operations like raw pointer dereferences or union
// field projections, we wouldn't know to require an `unsafe` block
// around a `match` equivalent to `std::intrinsics::unreachable()`.
// See issue #47412 for this hole being discovered in the wild.
//
// HACK(eddyb) Work around the above issue by adding a dummy inspection
// of `scrutinee_place`, specifically by applying `ReadForMatch`.
//
// NOTE: ReadForMatch also checks that the scrutinee is initialized.
// This is currently needed to not allow matching on an uninitialized,
// uninhabited value. If we get never patterns, those will check that
// the place is initialized, and so this read would only be used to
// check safety.
let cause_matched_place = FakeReadCause::ForMatchedPlace(None);

if let Some(scrutinee_place) = scrutinee_place_builder.try_to_place(self) {
self.cfg.push_fake_read(
otherwise_block,
source_info,
cause_matched_place,
scrutinee_place,
);
}
// See the doc comment on `match_candidates` for why we may have an
// otherwise block. Match checking will ensure this is actually
// unreachable.
let source_info = self.source_info(scrutinee_span);

// Matching on a `scrutinee_place` with an uninhabited type doesn't
// generate any memory reads by itself, and so if the place "expression"
// contains unsafe operations like raw pointer dereferences or union
// field projections, we wouldn't know to require an `unsafe` block
// around a `match` equivalent to `std::intrinsics::unreachable()`.
// See issue #47412 for this hole being discovered in the wild.
//
// HACK(eddyb) Work around the above issue by adding a dummy inspection
// of `scrutinee_place`, specifically by applying `ReadForMatch`.
//
// NOTE: ReadForMatch also checks that the scrutinee is initialized.
// This is currently needed to not allow matching on an uninitialized,
// uninhabited value. If we get never patterns, those will check that
// the place is initialized, and so this read would only be used to
// check safety.
let cause_matched_place = FakeReadCause::ForMatchedPlace(None);

self.cfg.terminate(otherwise_block, source_info, TerminatorKind::Unreachable);
if let Some(scrutinee_place) = scrutinee_place_builder.try_to_place(self) {
self.cfg.push_fake_read(
otherwise_block,
source_info,
cause_matched_place,
scrutinee_place,
);
}

self.cfg.terminate(otherwise_block, source_info, TerminatorKind::Unreachable);

// Link each leaf candidate to the `pre_binding_block` of the next one.
let mut previous_candidate: Option<&mut Candidate<'_, '_>> = None;

Expand Down Expand Up @@ -1163,7 +1161,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
span: Span,
scrutinee_span: Span,
start_block: BasicBlock,
otherwise_block: &mut Option<BasicBlock>,
otherwise_block: BasicBlock,
candidates: &mut [&mut Candidate<'pat, 'tcx>],
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) {
Expand Down Expand Up @@ -1210,7 +1208,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
span: Span,
scrutinee_span: Span,
start_block: BasicBlock,
otherwise_block: &mut Option<BasicBlock>,
otherwise_block: BasicBlock,
candidates: &mut [&mut Candidate<'_, 'tcx>],
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) {
Expand Down Expand Up @@ -1243,11 +1241,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// never reach this point.
if unmatched_candidates.is_empty() {
let source_info = self.source_info(span);
if let Some(otherwise) = *otherwise_block {
self.cfg.goto(block, source_info, otherwise);
} else {
*otherwise_block = Some(block);
}
self.cfg.goto(block, source_info, otherwise_block);
return;
}

Expand Down Expand Up @@ -1428,7 +1422,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
scrutinee_span: Span,
candidates: &mut [&mut Candidate<'_, 'tcx>],
block: BasicBlock,
otherwise_block: &mut Option<BasicBlock>,
otherwise_block: BasicBlock,
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) {
let (first_candidate, remaining_candidates) = candidates.split_first_mut().unwrap();
Expand All @@ -1453,7 +1447,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let match_pairs = mem::take(&mut first_candidate.match_pairs);
first_candidate.pre_binding_block = Some(block);

let mut otherwise = None;
let remainder_start = self.cfg.start_new_block();
for match_pair in match_pairs {
let PatKind::Or { ref pats } = &match_pair.pattern.kind else {
bug!("Or-patterns should have been sorted to the end");
Expand All @@ -1463,7 +1457,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
first_candidate.visit_leaves(|leaf_candidate| {
self.test_or_pattern(
leaf_candidate,
&mut otherwise,
remainder_start,
pats,
or_span,
&match_pair.place,
Expand All @@ -1472,8 +1466,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
});
}

let remainder_start = otherwise.unwrap_or_else(|| self.cfg.start_new_block());

self.match_candidates(
span,
scrutinee_span,
Expand All @@ -1491,7 +1483,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fn test_or_pattern<'pat>(
&mut self,
candidate: &mut Candidate<'pat, 'tcx>,
otherwise: &mut Option<BasicBlock>,
otherwise: BasicBlock,
pats: &'pat [Box<Pat<'tcx>>],
or_span: Span,
place: &PlaceBuilder<'tcx>,
Expand All @@ -1503,8 +1495,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
.map(|pat| Candidate::new(place.clone(), pat, candidate.has_guard, self))
.collect();
let mut or_candidate_refs: Vec<_> = or_candidates.iter_mut().collect();
let otherwise = if candidate.otherwise_block.is_some() {
&mut candidate.otherwise_block
let otherwise = if let Some(otherwise_block) = candidate.otherwise_block {
otherwise_block
} else {
otherwise
};
Expand Down Expand Up @@ -1680,8 +1672,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
span: Span,
scrutinee_span: Span,
mut candidates: &'b mut [&'c mut Candidate<'pat, 'tcx>],
block: BasicBlock,
otherwise_block: &mut Option<BasicBlock>,
start_block: BasicBlock,
otherwise_block: BasicBlock,
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) {
// extract the match-pair from the highest priority candidate
Expand Down Expand Up @@ -1749,12 +1741,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
debug!("untested_candidates: {}", candidates.len());

// The block that we should branch to if none of the
// `target_candidates` match. This is either the block where we
// start matching the untested candidates if there are any,
// otherwise it's the `otherwise_block`.
let remainder_start = &mut None;
let remainder_start =
if candidates.is_empty() { &mut *otherwise_block } else { remainder_start };
// `target_candidates` match.
let remainder_start = self.cfg.start_new_block();

// For each outcome of test, process the candidates that still
// apply. Collect a list of blocks where control flow will
Expand All @@ -1775,13 +1763,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
);
candidate_start
} else {
*remainder_start.get_or_insert_with(|| self.cfg.start_new_block())
remainder_start
}
})
.collect();

if !candidates.is_empty() {
let remainder_start = remainder_start.unwrap_or_else(|| self.cfg.start_new_block());
self.match_candidates(
span,
scrutinee_span,
Expand All @@ -1790,9 +1777,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
candidates,
fake_borrows,
);
} else {
let source_info = self.source_info(span);
self.cfg.goto(remainder_start, source_info, otherwise_block);
}

self.perform_test(span, scrutinee_span, block, &match_place, &test, target_blocks);
// Perform the test, branching to one of N blocks.
self.perform_test(span, scrutinee_span, start_block, &match_place, &test, target_blocks);
}

/// Determine the fake borrows that are needed from a set of places that
Expand Down
14 changes: 7 additions & 7 deletions tests/mir-opt/building/async_await.b-{closure#0}.coroutine_resume.0.mir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ fn b::{closure#0}(_1: Pin<&mut {async fn body@$DIR/async_await.rs:15:18: 18:2}>,

bb0: {
_39 = discriminant((*(_1.0: &mut {async fn body@$DIR/async_await.rs:15:18: 18:2})));
switchInt(move _39) -> [0: bb1, 1: bb29, 3: bb27, 4: bb28, otherwise: bb9];
switchInt(move _39) -> [0: bb1, 1: bb29, 3: bb27, 4: bb28, otherwise: bb8];
}

bb1: {
Expand Down Expand Up @@ -165,10 +165,14 @@ fn b::{closure#0}(_1: Pin<&mut {async fn body@$DIR/async_await.rs:15:18: 18:2}>,
StorageDead(_10);
PlaceMention(_9);
_16 = discriminant(_9);
switchInt(move _16) -> [0: bb10, 1: bb8, otherwise: bb9];
switchInt(move _16) -> [0: bb10, 1: bb9, otherwise: bb8];
}

bb8: {
unreachable;
}

bb9: {
_8 = const ();
StorageDead(_14);
StorageDead(_12);
Expand All @@ -186,10 +190,6 @@ fn b::{closure#0}(_1: Pin<&mut {async fn body@$DIR/async_await.rs:15:18: 18:2}>,
return;
}

bb9: {
unreachable;
}

bb10: {
StorageLive(_17);
_17 = ((_9 as Ready).0: ());
Expand Down Expand Up @@ -267,7 +267,7 @@ fn b::{closure#0}(_1: Pin<&mut {async fn body@$DIR/async_await.rs:15:18: 18:2}>,
StorageDead(_26);
PlaceMention(_25);
_32 = discriminant(_25);
switchInt(move _32) -> [0: bb21, 1: bb20, otherwise: bb9];
switchInt(move _32) -> [0: bb21, 1: bb20, otherwise: bb8];
}

bb20: {
Expand Down
17 changes: 11 additions & 6 deletions tests/mir-opt/building/issue_101867.main.built.after.mir
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ fn main() -> () {
StorageLive(_5);
PlaceMention(_1);
_6 = discriminant(_1);
switchInt(move _6) -> [1: bb4, otherwise: bb3];
switchInt(move _6) -> [1: bb5, otherwise: bb4];
}

bb1: {
StorageLive(_3);
StorageLive(_4);
_4 = begin_panic::<&str>(const "explicit panic") -> bb7;
_4 = begin_panic::<&str>(const "explicit panic") -> bb8;
}

bb2: {
Expand All @@ -43,27 +43,32 @@ fn main() -> () {
}

bb3: {
goto -> bb6;
FakeRead(ForMatchedPlace(None), _1);
unreachable;
}

bb4: {
falseEdge -> [real: bb5, imaginary: bb3];
goto -> bb7;
}

bb5: {
falseEdge -> [real: bb6, imaginary: bb4];
}

bb6: {
_5 = ((_1 as Some).0: u8);
_0 = const ();
StorageDead(_5);
StorageDead(_1);
return;
}

bb6: {
bb7: {
StorageDead(_5);
goto -> bb1;
}

bb7 (cleanup): {
bb8 (cleanup): {
resume;
}
}
Loading

0 comments on commit 61edca9

Please sign in to comment.