Skip to content

Commit

Permalink
Auto merge of #67668 - matthewjasper:or-patterns, r=pnkfelix
Browse files Browse the repository at this point in the history
Implement MIR lowering for or-patterns

This is the last thing needed to get meaningful run-pass tests for or-patterns. There probably need to be more tests before stabilizing this, but the most important cases should have been covered.

Note: we can generate exponentially large MIR CFGs when using or-patterns containing bindings, type ascriptions, or that are for a match arm with a guard. `src/test/mir-opt/exponential-or.rs` shows the best case for what we currently do.

cc #54883
closes #60350
closes #67514

cc @Centril
r? @pnkfelix
  • Loading branch information
bors committed Feb 3, 2020
2 parents 8417d68 + 8dbbe4d commit 42a0bd2
Show file tree
Hide file tree
Showing 65 changed files with 1,737 additions and 894 deletions.
1 change: 0 additions & 1 deletion src/librustc_feature/active.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,6 @@ pub const INCOMPLETE_FEATURES: &[Symbol] = &[
sym::impl_trait_in_bindings,
sym::generic_associated_types,
sym::const_generics,
sym::or_patterns,
sym::let_chains,
sym::raw_dylib,
sym::const_trait_impl,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir_build/build/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
&pattern,
UserTypeProjections::none(),
&mut |this, _, _, _, node, span, _, _| {
this.storage_live_binding(block, node, span, OutsideGuard);
this.storage_live_binding(block, node, span, OutsideGuard, true);
this.schedule_drop_for_binding(node, span, OutsideGuard);
},
)
Expand Down
850 changes: 588 additions & 262 deletions src/librustc_mir_build/build/matches/mod.rs

Large diffs are not rendered by default.

52 changes: 50 additions & 2 deletions src/librustc_mir_build/build/matches/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::build::matches::{Ascription, Binding, Candidate, MatchPair};
use crate::build::Builder;
use crate::hair::{self, *};
use rustc::mir::interpret::truncate;
use rustc::mir::Place;
use rustc::ty;
use rustc::ty::layout::{Integer, IntegerExt, Size};
use rustc_attr::{SignedInt, UnsignedInt};
Expand All @@ -24,10 +25,33 @@ use rustc_hir::RangeEnd;
use std::mem;

impl<'a, 'tcx> Builder<'a, 'tcx> {
crate fn simplify_candidate<'pat>(&mut self, candidate: &mut Candidate<'pat, 'tcx>) {
/// Simplify a candidate so that all match pairs require a test.
///
/// This method will also split a candidate where the only match-pair is an
/// or-pattern into multiple candidates. This is so that
///
/// match x {
/// 0 | 1 => { ... },
/// 2 | 3 => { ... },
/// }
///
/// only generates a single switch. If this happens this method returns
/// `true`.
pub(super) fn simplify_candidate<'pat>(
&mut self,
candidate: &mut Candidate<'pat, 'tcx>,
) -> bool {
// repeatedly simplify match pairs until fixed point is reached
loop {
let match_pairs = mem::take(&mut candidate.match_pairs);

if let [MatchPair { pattern: Pat { kind: box PatKind::Or { pats }, .. }, place }] =
*match_pairs
{
candidate.subcandidates = self.create_or_subcandidates(candidate, place, pats);
return true;
}

let mut changed = false;
for match_pair in match_pairs {
match self.simplify_match_pair(match_pair, candidate) {
Expand All @@ -40,11 +64,35 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}
if !changed {
return; // if we were not able to simplify any, done.
// Move or-patterns to the end, because they can result in us
// creating additional candidates, so we want to test them as
// late as possible.
candidate
.match_pairs
.sort_by_key(|pair| matches!(*pair.pattern.kind, PatKind::Or { .. }));
return false; // if we were not able to simplify any, done.
}
}
}

/// Given `candidate` that has a single or-pattern for its match-pairs,
/// creates a fresh candidate for each of its input subpatterns passed via
/// `pats`.
fn create_or_subcandidates<'pat>(
&mut self,
candidate: &Candidate<'pat, 'tcx>,
place: Place<'tcx>,
pats: &'pat [Pat<'tcx>],
) -> Vec<Candidate<'pat, 'tcx>> {
pats.iter()
.map(|pat| {
let mut candidate = Candidate::new(place, pat, candidate.has_guard);
self.simplify_candidate(&mut candidate);
candidate
})
.collect()
}

/// Tries to simplify `match_pair`, returning `Ok(())` if
/// successful. If successful, new match pairs and bindings will
/// have been pushed into the candidate. If no simplification is
Expand Down
42 changes: 16 additions & 26 deletions src/librustc_mir_build/build/matches/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// Identifies what test is needed to decide if `match_pair` is applicable.
///
/// It is a bug to call this with a simplifiable pattern.
crate fn test<'pat>(&mut self, match_pair: &MatchPair<'pat, 'tcx>) -> Test<'tcx> {
pub(super) fn test<'pat>(&mut self, match_pair: &MatchPair<'pat, 'tcx>) -> Test<'tcx> {
match *match_pair.pattern.kind {
PatKind::Variant { ref adt_def, substs: _, variant_index: _, subpatterns: _ } => Test {
span: match_pair.pattern.span,
Expand Down Expand Up @@ -70,11 +70,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

PatKind::Or { .. } => self
.hir
.tcx()
.sess
.span_fatal(match_pair.pattern.span, "or-patterns are not fully implemented yet"),
PatKind::Or { .. } => bug!("or-patterns should have already been handled"),

PatKind::AscribeUserType { .. }
| PatKind::Array { .. }
Expand All @@ -85,7 +81,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

crate fn add_cases_to_switch<'pat>(
pub(super) fn add_cases_to_switch<'pat>(
&mut self,
test_place: &Place<'tcx>,
candidate: &Candidate<'pat, 'tcx>,
Expand Down Expand Up @@ -129,7 +125,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

crate fn add_variants_to_switch<'pat>(
pub(super) fn add_variants_to_switch<'pat>(
&mut self,
test_place: &Place<'tcx>,
candidate: &Candidate<'pat, 'tcx>,
Expand All @@ -156,10 +152,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
}
}

crate fn perform_test(
pub(super) fn perform_test(
&mut self,
block: BasicBlock,
place: &Place<'tcx>,
place: Place<'tcx>,
test: &Test<'tcx>,
make_target_blocks: impl FnOnce(&mut Self) -> Vec<BasicBlock>,
) {
Expand Down Expand Up @@ -209,7 +205,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
);
let discr_ty = adt_def.repr.discr_type().to_ty(tcx);
let discr = self.temp(discr_ty, test.span);
self.cfg.push_assign(block, source_info, &discr, Rvalue::Discriminant(*place));
self.cfg.push_assign(block, source_info, &discr, Rvalue::Discriminant(place));
assert_eq!(values.len() + 1, targets.len());
self.cfg.terminate(
block,
Expand All @@ -233,20 +229,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
0 => (second_bb, first_bb),
v => span_bug!(test.span, "expected boolean value but got {:?}", v),
};
TerminatorKind::if_(
self.hir.tcx(),
Operand::Copy(*place),
true_bb,
false_bb,
)
TerminatorKind::if_(self.hir.tcx(), Operand::Copy(place), true_bb, false_bb)
} else {
bug!("`TestKind::SwitchInt` on `bool` should have two targets")
}
} else {
// The switch may be inexhaustive so we have a catch all block
debug_assert_eq!(options.len() + 1, target_blocks.len());
TerminatorKind::SwitchInt {
discr: Operand::Copy(*place),
discr: Operand::Copy(place),
switch_ty,
values: options.clone().into(),
targets: target_blocks,
Expand All @@ -271,7 +262,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
if let [success, fail] = *make_target_blocks(self) {
assert_eq!(value.ty, ty);
let expect = self.literal_operand(test.span, value);
let val = Operand::Copy(*place);
let val = Operand::Copy(place);
self.compare(block, success, fail, source_info, BinOp::Eq, expect, val);
} else {
bug!("`TestKind::Eq` should have two target blocks");
Expand All @@ -286,7 +277,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// Test `val` by computing `lo <= val && val <= hi`, using primitive comparisons.
let lo = self.literal_operand(test.span, lo);
let hi = self.literal_operand(test.span, hi);
let val = Operand::Copy(*place);
let val = Operand::Copy(place);

if let [success, fail] = *target_blocks {
self.compare(
Expand Down Expand Up @@ -315,7 +306,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let actual = self.temp(usize_ty, test.span);

// actual = len(place)
self.cfg.push_assign(block, source_info, &actual, Rvalue::Len(*place));
self.cfg.push_assign(block, source_info, &actual, Rvalue::Len(place));

// expected = <N>
let expected = self.push_usize(block, source_info, len);
Expand Down Expand Up @@ -371,13 +362,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
make_target_blocks: impl FnOnce(&mut Self) -> Vec<BasicBlock>,
source_info: SourceInfo,
value: &'tcx ty::Const<'tcx>,
place: &Place<'tcx>,
place: Place<'tcx>,
mut ty: Ty<'tcx>,
) {
use rustc::middle::lang_items::EqTraitLangItem;

let mut expect = self.literal_operand(source_info.span, value);
let mut val = Operand::Copy(*place);
let mut val = Operand::Copy(place);

// If we're using `b"..."` as a pattern, we need to insert an
// unsizing coercion, as the byte string has the type `&[u8; N]`.
Expand Down Expand Up @@ -502,7 +493,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// that it *doesn't* apply. For now, we return false, indicate that the
/// test does not apply to this candidate, but it might be we can get
/// tighter match code if we do something a bit different.
crate fn sort_candidate<'pat>(
pub(super) fn sort_candidate<'pat>(
&mut self,
test_place: &Place<'tcx>,
test: &Test<'tcx>,
Expand Down Expand Up @@ -755,8 +746,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let downcast_place = tcx.mk_place_elem(match_pair.place, elem); // `(x as Variant)`
let consequent_match_pairs = subpatterns.iter().map(|subpattern| {
// e.g., `(x as Variant).0`
let place =
tcx.mk_place_field(downcast_place.clone(), subpattern.field, subpattern.pattern.ty);
let place = tcx.mk_place_field(downcast_place, subpattern.field, subpattern.pattern.ty);
// e.g., `(x as Variant).0 @ P1`
MatchPair::new(place, &subpattern.pattern)
});
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir_build/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -899,7 +899,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
matches::ArmHasGuard(false),
Some((Some(&place), span)),
);
unpack!(block = self.place_into_pattern(block, pattern, &place, false));
unpack!(block = self.place_into_pattern(block, pattern, place, false));
}
}
self.source_scope = original_source_scope;
Expand Down
11 changes: 0 additions & 11 deletions src/librustc_mir_build/hair/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,17 +315,6 @@ crate struct Arm<'tcx> {
crate span: Span,
}

impl<'tcx> Arm<'tcx> {
// HACK(or_patterns; Centril | dlrobertson): Remove this and
// correctly handle each case in which this method is used.
crate fn top_pats_hack(&self) -> &[Pat<'tcx>] {
match &*self.pattern.kind {
PatKind::Or { pats } => pats,
_ => std::slice::from_ref(&self.pattern),
}
}
}

#[derive(Clone, Debug)]
crate enum Guard<'tcx> {
If(ExprRef<'tcx>),
Expand Down
20 changes: 10 additions & 10 deletions src/test/mir-opt/const_prop/discriminant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,18 @@ fn main() {
// ...
// _3 = std::option::Option::<bool>::Some(const true,);
// _4 = discriminant(_3);
// switchInt(move _4) -> [1isize: bb3, otherwise: bb2];
// switchInt(move _4) -> [1isize: bb2, otherwise: bb1];
// }
// bb1: {
// _2 = const 42i32;
// _2 = const 10i32;
// goto -> bb4;
// }
// bb2: {
// _2 = const 10i32;
// goto -> bb4;
// switchInt(((_3 as Some).0: bool)) -> [false: bb1, otherwise: bb3];
// }
// bb3: {
// switchInt(((_3 as Some).0: bool)) -> [false: bb2, otherwise: bb1];
// _2 = const 42i32;
// goto -> bb4;
// }
// bb4: {
// _1 = Add(move _2, const 0i32);
Expand All @@ -33,18 +33,18 @@ fn main() {
// ...
// _3 = const Scalar(0x01) : std::option::Option<bool>;
// _4 = const 1isize;
// switchInt(const 1isize) -> [1isize: bb3, otherwise: bb2];
// switchInt(const 1isize) -> [1isize: bb2, otherwise: bb1];
// }
// bb1: {
// _2 = const 42i32;
// _2 = const 10i32;
// goto -> bb4;
// }
// bb2: {
// _2 = const 10i32;
// goto -> bb4;
// switchInt(const true) -> [false: bb1, otherwise: bb3];
// }
// bb3: {
// switchInt(const true) -> [false: bb2, otherwise: bb1];
// _2 = const 42i32;
// goto -> bb4;
// }
// bb4: {
// _1 = Add(move _2, const 0i32);
Expand Down
76 changes: 76 additions & 0 deletions src/test/mir-opt/exponential-or.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Test that simple or-patterns don't get expanded to exponentially large CFGs

// ignore-tidy-linelength

#![feature(or_patterns)]

fn match_tuple(x: (u32, bool, Option<i32>, u32)) -> u32 {
match x {
(y @ (1 | 4), true | false, Some(1 | 8) | None, z @ (6..=9 | 13..=16)) => y ^ z,
_ => 0,
}
}

fn main() {}

// END RUST SOURCE

// START rustc.match_tuple.SimplifyCfg-initial.after.mir
// scope 1 {
// debug y => _7;
// debug z => _8;
// }
// bb0: {
// FakeRead(ForMatchedPlace, _1);
// switchInt((_1.0: u32)) -> [1u32: bb2, 4u32: bb2, otherwise: bb1];
// }
// bb1: {
// _0 = const 0u32;
// goto -> bb10;
// }
// bb2: {
// _2 = discriminant((_1.2: std::option::Option<i32>));
// switchInt(move _2) -> [0isize: bb4, 1isize: bb3, otherwise: bb1];
// }
// bb3: {
// switchInt((((_1.2: std::option::Option<i32>) as Some).0: i32)) -> [1i32: bb4, 8i32: bb4, otherwise: bb1];
// }
// bb4: {
// _5 = Le(const 6u32, (_1.3: u32));
// switchInt(move _5) -> [false: bb6, otherwise: bb5];
// }
// bb5: {
// _6 = Le((_1.3: u32), const 9u32);
// switchInt(move _6) -> [false: bb6, otherwise: bb8];
// }
// bb6: {
// _3 = Le(const 13u32, (_1.3: u32));
// switchInt(move _3) -> [false: bb1, otherwise: bb7];
// }
// bb7: {
// _4 = Le((_1.3: u32), const 16u32);
// switchInt(move _4) -> [false: bb1, otherwise: bb8];
// }
// bb8: {
// falseEdges -> [real: bb9, imaginary: bb1];
// }
// bb9: {
// StorageLive(_7);
// _7 = (_1.0: u32);
// StorageLive(_8);
// _8 = (_1.3: u32);
// StorageLive(_9);
// _9 = _7;
// StorageLive(_10);
// _10 = _8;
// _0 = BitXor(move _9, move _10);
// StorageDead(_10);
// StorageDead(_9);
// StorageDead(_8);
// StorageDead(_7);
// goto -> bb10;
// }
// bb10: {
// return;
// }
// END rustc.match_tuple.SimplifyCfg-initial.after.mir
Loading

0 comments on commit 42a0bd2

Please sign in to comment.