mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Don't use fake wildcards when we can get the failure block directly

Browse Source 
This commit too was obtained by repeatedly inlining and simplifying.
This commit is contained in:
Nadrieril 2024-06-17 20:16:53 +02:00
parent c0c6c32a45
commit 7b150a161e
3 changed files with 130 additions and 148 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

109
compiler/rustc_mir_build/src/build/matches/mod.rs
View File

@ -329,6 +329,7 @@ pub(crate) fn match_expr(
&scrutinee_place,
match_start_span,
&mut candidates,
false,
);
self.lower_match_arms(
@ -691,6 +692,7 @@ pub(crate) fn place_into_pattern(
&initializer,
irrefutable_pat.span,
&mut [&mut candidate],
false,
);
self.bind_pattern(
self.source_info(irrefutable_pat.span),
@ -1215,6 +1217,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
///
/// Modifies `candidates` to store the bindings and type ascriptions for
/// that candidate.
///
/// `refutable` indicates whether the candidate list is refutable (for `if let` and `let else`)
/// or not (for `let` and `match`). In the refutable case we return the block to which we branch
/// on failure.
fn lower_match_tree<'pat>(
&mut self,
block: BasicBlock,
@ -1222,45 +1228,14 @@ fn lower_match_tree<'pat>(
scrutinee_place_builder: &PlaceBuilder<'tcx>,
match_start_span: Span,
candidates: &mut [&mut Candidate<'pat, 'tcx>],
) {
// See the doc comment on `match_candidates` for why we have an
// otherwise block. Match checking will ensure this is actually
// unreachable.
refutable: bool,
) -> BasicBlock {
// See the doc comment on `match_candidates` for why we have an otherwise block.
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, otherwise_block, candidates);
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,
);
}
self.cfg.terminate(otherwise_block, source_info, TerminatorKind::Unreachable);
// Link each leaf candidate to the `false_edge_start_block` of the next one.
let mut previous_candidate: Option<&mut Candidate<'_, '_>> = None;
for candidate in candidates {
@ -1272,6 +1247,46 @@ fn lower_match_tree<'pat>(
previous_candidate = Some(leaf_candidate);
});
}
if refutable {
// In refutable cases there's always at least one candidate, and we want a false edge to
// the failure block.
previous_candidate.as_mut().unwrap().next_candidate_start_block = Some(otherwise_block)
} else {
// Match checking ensures `otherwise_block` is actually unreachable in irrefutable
// cases.
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,
);
}
self.cfg.terminate(otherwise_block, source_info, TerminatorKind::Unreachable);
}
otherwise_block
}
/// The main match algorithm. It begins with a set of candidates
@ -1978,21 +1993,18 @@ pub(crate) fn lower_let_expr(
) -> BlockAnd<()> {
let expr_span = self.thir[expr_id].span;
let expr_place_builder = unpack!(block = self.lower_scrutinee(block, expr_id, expr_span));
let wildcard = Pat::wildcard_from_ty(pat.ty);
let mut guard_candidate = Candidate::new(expr_place_builder.clone(), pat, false, self);
let mut otherwise_candidate =
Candidate::new(expr_place_builder.clone(), &wildcard, false, self);
self.lower_match_tree(
let otherwise_block = self.lower_match_tree(
block,
pat.span,
&expr_place_builder,
pat.span,
&mut [&mut guard_candidate, &mut otherwise_candidate],
&mut [&mut guard_candidate],
true,
);
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, self.source_info(expr_span));
self.break_for_else(otherwise_block, self.source_info(expr_span));
if declare_bindings {
self.declare_bindings(source_scope, pat.span.to(span), pat, None, opt_expr_place);
@ -2008,7 +2020,7 @@ pub(crate) fn lower_let_expr(
);
// If branch coverage is enabled, record this branch.
self.visit_coverage_conditional_let(pat, post_guard_block, otherwise_post_guard_block);
self.visit_coverage_conditional_let(pat, post_guard_block, otherwise_block);
post_guard_block.unit()
}
@ -2470,15 +2482,14 @@ pub(crate) fn ast_let_else(
let else_block_span = self.thir[else_block].span;
let (matching, failure) = self.in_if_then_scope(*let_else_scope, else_block_span, |this| {
let scrutinee = unpack!(block = this.lower_scrutinee(block, init_id, initializer_span));
let pat = Pat { ty: pattern.ty, span: else_block_span, kind: PatKind::Wild };
let mut wildcard = Candidate::new(scrutinee.clone(), &pat, false, this);
let mut candidate = Candidate::new(scrutinee.clone(), pattern, false, this);
this.lower_match_tree(
let failure_block = this.lower_match_tree(
block,
initializer_span,
&scrutinee,
pattern.span,
&mut [&mut candidate, &mut wildcard],
&mut [&mut candidate],
true,
);
// This block is for the matching case
let matching = this.bind_pattern(
@ -2489,13 +2500,11 @@ pub(crate) fn ast_let_else(
None,
true,
);
// This block is for the failure case
let failure = wildcard.pre_binding_block.unwrap();
// If branch coverage is enabled, record this branch.
this.visit_coverage_conditional_let(pattern, matching, failure);
this.visit_coverage_conditional_let(pattern, matching, failure_block);
this.break_for_else(failure, this.source_info(initializer_span));
this.break_for_else(failure_block, this.source_info(initializer_span));
matching.unit()
});
matching.and(failure)

21
tests/mir-opt/building/issue_101867.main.built.after.mir
View File

@ -27,13 +27,13 @@ fn main() -> () {
StorageLive(_5);
PlaceMention(_1);
_6 = discriminant(_1);
switchInt(move _6) -> [1: bb6, otherwise: bb4];
switchInt(move _6) -> [1: bb4, otherwise: bb3];
}
bb1: {
StorageLive(_3);
StorageLive(_4);
_4 = begin_panic::<&str>(const "explicit panic") -> bb10;
_4 = begin_panic::<&str>(const "explicit panic") -> bb8;
}
bb2: {
@ -43,12 +43,11 @@ fn main() -> () {
}
bb3: {
FakeRead(ForMatchedPlace(None), _1);
unreachable;
goto -> bb7;
}
bb4: {
goto -> bb9;
falseEdge -> [real: bb6, imaginary: bb3];
}
bb5: {
@ -56,14 +55,6 @@ fn main() -> () {
}
bb6: {
falseEdge -> [real: bb8, imaginary: bb4];
}
bb7: {
goto -> bb4;
}
bb8: {
_5 = ((_1 as Some).0: u8);
_0 = const ();
StorageDead(_5);
@ -71,12 +62,12 @@ fn main() -> () {
return;
}
bb9: {
bb7: {
StorageDead(_5);
goto -> bb1;
}
bb10 (cleanup): {
bb8 (cleanup): {
resume;
}
}

148
tests/mir-opt/building/logical_or_in_conditional.test_complex.built.after.mir
View File

@ -19,22 +19,21 @@ fn test_complex() -> () {
bb0: {
StorageLive(_1);
StorageLive(_2);
_2 = E::f() -> [return: bb1, unwind: bb38];
_2 = E::f() -> [return: bb1, unwind: bb34];
}
bb1: {
PlaceMention(_2);
_3 = discriminant(_2);
switchInt(move _3) -> [0: bb5, otherwise: bb3];
switchInt(move _3) -> [0: bb3, otherwise: bb2];
}
bb2: {
FakeRead(ForMatchedPlace(None), _2);
unreachable;
goto -> bb21;
}
bb3: {
goto -> bb23;
falseEdge -> [real: bb5, imaginary: bb2];
}
bb4: {
@ -42,175 +41,158 @@ fn test_complex() -> () {
}
bb5: {
falseEdge -> [real: bb7, imaginary: bb3];
StorageLive(_4);
_4 = always_true() -> [return: bb6, unwind: bb34];
}
bb6: {
goto -> bb3;
switchInt(move _4) -> [0: bb8, otherwise: bb7];
}
bb7: {
StorageLive(_4);
_4 = always_true() -> [return: bb8, unwind: bb38];
}
bb8: {
switchInt(move _4) -> [0: bb10, otherwise: bb9];
}
bb9: {
StorageLive(_5);
StorageLive(_6);
StorageLive(_7);
_7 = Droppy(const 0_u8);
_6 = (_7.0: u8);
_5 = Gt(move _6, const 0_u8);
switchInt(move _5) -> [0: bb12, otherwise: bb11];
switchInt(move _5) -> [0: bb10, otherwise: bb9];
}
bb8: {
goto -> bb14;
}
bb9: {
drop(_7) -> [return: bb11, unwind: bb34];
}
bb10: {
goto -> bb16;
goto -> bb12;
}
bb11: {
drop(_7) -> [return: bb13, unwind: bb38];
StorageDead(_7);
StorageDead(_6);
goto -> bb18;
}
bb12: {
goto -> bb14;
drop(_7) -> [return: bb13, unwind: bb34];
}
bb13: {
StorageDead(_7);
StorageDead(_6);
goto -> bb20;
goto -> bb14;
}
bb14: {
drop(_7) -> [return: bb15, unwind: bb38];
}
bb15: {
StorageDead(_7);
StorageDead(_6);
goto -> bb16;
}
bb16: {
StorageLive(_8);
StorageLive(_9);
StorageLive(_10);
_10 = Droppy(const 1_u8);
_9 = (_10.0: u8);
_8 = Gt(move _9, const 1_u8);
switchInt(move _8) -> [0: bb18, otherwise: bb17];
switchInt(move _8) -> [0: bb16, otherwise: bb15];
}
bb15: {
drop(_10) -> [return: bb17, unwind: bb34];
}
bb16: {
goto -> bb19;
}
bb17: {
drop(_10) -> [return: bb19, unwind: bb38];
StorageDead(_10);
StorageDead(_9);
goto -> bb18;
}
bb18: {
goto -> bb21;
_1 = const ();
goto -> bb22;
}
bb19: {
StorageDead(_10);
StorageDead(_9);
goto -> bb20;
drop(_10) -> [return: bb20, unwind: bb34];
}
bb20: {
_1 = const ();
goto -> bb24;
StorageDead(_10);
StorageDead(_9);
goto -> bb21;
}
bb21: {
drop(_10) -> [return: bb22, unwind: bb38];
_1 = const ();
goto -> bb22;
}
bb22: {
StorageDead(_10);
StorageDead(_9);
goto -> bb23;
}
bb23: {
_1 = const ();
goto -> bb24;
}
bb24: {
StorageDead(_8);
StorageDead(_5);
StorageDead(_4);
StorageDead(_2);
StorageDead(_1);
StorageLive(_11);
_11 = always_true() -> [return: bb25, unwind: bb38];
_11 = always_true() -> [return: bb23, unwind: bb34];
}
bb23: {
switchInt(move _11) -> [0: bb25, otherwise: bb24];
}
bb24: {
goto -> bb32;
}
bb25: {
switchInt(move _11) -> [0: bb27, otherwise: bb26];
goto -> bb26;
}
bb26: {
goto -> bb36;
StorageLive(_12);
_12 = E::f() -> [return: bb27, unwind: bb34];
}
bb27: {
goto -> bb28;
PlaceMention(_12);
_13 = discriminant(_12);
switchInt(move _13) -> [1: bb29, otherwise: bb28];
}
bb28: {
StorageLive(_12);
_12 = E::f() -> [return: bb29, unwind: bb38];
goto -> bb32;
}
bb29: {
PlaceMention(_12);
_13 = discriminant(_12);
switchInt(move _13) -> [1: bb33, otherwise: bb31];
falseEdge -> [real: bb31, imaginary: bb28];
}
bb30: {
FakeRead(ForMatchedPlace(None), _12);
unreachable;
goto -> bb28;
}
bb31: {
goto -> bb36;
_0 = const ();
goto -> bb33;
}
bb32: {
goto -> bb30;
_0 = const ();
goto -> bb33;
}
bb33: {
falseEdge -> [real: bb35, imaginary: bb31];
}
bb34: {
goto -> bb31;
}
bb35: {
_0 = const ();
goto -> bb37;
}
bb36: {
_0 = const ();
goto -> bb37;
}
bb37: {
StorageDead(_11);
StorageDead(_12);
return;
}
bb38 (cleanup): {
bb34 (cleanup): {
resume;
}
}