Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change match desugaring to degenerate to if-else-if chains #29776

Merged
merged 2 commits into from
Nov 12, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
207 changes: 184 additions & 23 deletions src/librustc_mir/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,15 @@ impl<'a,'tcx> Builder<'a,'tcx> {

// this will generate code to test discriminant_lvalue and
// branch to the appropriate arm block
self.match_candidates(span, &mut arm_blocks, candidates, block);
let otherwise = self.match_candidates(span, &mut arm_blocks, candidates, block);

// because all matches are exhaustive, in principle we expect
// an empty vector to be returned here, but the algorithm is
// not entirely precise
if !otherwise.is_empty() {
let join_block = self.join_otherwise_blocks(otherwise);
self.panic(join_block);
}

// all the arm blocks will rejoin here
let end_block = self.cfg.start_new_block();
Expand Down Expand Up @@ -279,11 +287,32 @@ struct Test<'tcx> {
// Main matching algorithm

impl<'a,'tcx> Builder<'a,'tcx> {
/// The main match algorithm. It begins with a set of candidates
/// `candidates` and has the job of generating code to determine
/// which of these candidates, if any, is the correct one. The
/// candidates are sorted in inverse priority -- so the last item
/// in the list has highest priority. When a candidate is found to
/// match the value, we will generate a branch to the appropriate
/// block found in `arm_blocks`.
///
/// The return value is a list of "otherwise" blocks. These are
/// points in execution where we found that *NONE* of the
/// candidates apply. In principle, this means that the input
/// list was not exhaustive, though at present we sometimes are
/// not smart enough to recognize all exhaustive inputs.
///
/// It might be surprising that the input can be inexhaustive.
/// Indeed, initially, it is not, because all matches are
/// exhaustive in Rust. But during processing we sometimes divide
/// up the list of candidates and recurse with a non-exhaustive
/// list. This is important to keep the size of the generated code
/// under control. See `test_candidates` for more details.
fn match_candidates<'pat>(&mut self,
span: Span,
arm_blocks: &mut ArmBlocks,
mut candidates: Vec<Candidate<'pat, 'tcx>>,
mut block: BasicBlock)
-> Vec<BasicBlock>
{
debug!("matched_candidate(span={:?}, block={:?}, candidates={:?})",
span, block, candidates);
Expand Down Expand Up @@ -311,17 +340,127 @@ impl<'a,'tcx> Builder<'a,'tcx> {
} else {
// if None is returned, then any remaining candidates
// are unreachable (at least not through this path).
return;
return vec![];
}
}

// If there are no candidates that still need testing, we're done.
// Since all matches are exhaustive, execution should never reach this point.
if candidates.is_empty() {
return self.panic(block);
return vec![block];
}

// Test candidates where possible.
let (otherwise, tested_candidates) =
self.test_candidates(span, arm_blocks, &candidates, block);

// If the target candidates were exhaustive, then we are done.
if otherwise.is_empty() {
return vec![];
}

// If all candidates were sorted into `target_candidates` somewhere, then
// the initial set was inexhaustive.
let untested_candidates = candidates.len() - tested_candidates;
if untested_candidates == 0 {
return otherwise;
}

// otherwise, extract the next match pair and construct tests
// Otherwise, let's process those remaining candidates.
let join_block = self.join_otherwise_blocks(otherwise);
candidates.truncate(untested_candidates);
self.match_candidates(span, arm_blocks, candidates, join_block)
}

fn join_otherwise_blocks(&mut self,
otherwise: Vec<BasicBlock>)
-> BasicBlock
{
if otherwise.len() == 1 {
otherwise[0]
} else {
let join_block = self.cfg.start_new_block();
for block in otherwise {
self.cfg.terminate(block, Terminator::Goto { target: join_block });
}
join_block
}
}

/// This is the most subtle part of the matching algorithm. At
/// this point, the input candidates have been fully simplified,
/// and so we know that all remaining match-pairs require some
/// sort of test. To decide what test to do, we take the highest
/// priority candidate (last one in the list) and extract the
/// first match-pair from the list. From this we decide what kind
/// of test is needed using `test`, defined in the `test` module.
///
/// *Note:* taking the first match pair is somewhat arbitrary, and
/// we might do better here by choosing more carefully what to
/// test.
///
/// For example, consider the following possible match-pairs:
///
/// 1. `x @ Some(P)` -- we will do a `Switch` to decide what variant `x` has
/// 2. `x @ 22` -- we will do a `SwitchInt`
/// 3. `x @ 3..5` -- we will do a range test
/// 4. etc.
///
/// Once we know what sort of test we are going to perform, this
/// test may also help us with other candidates. So we walk over
/// the candidates (from high to low priority) and check. This
/// gives us, for each outcome of the test, a transformed list of
/// candidates. For example, if we are testing the current
/// variant of `x.0`, and we have a candidate `{x.0 @ Some(v), x.1
/// @ 22}`, then we would have a resulting candidate of `{(x.0 as
/// Some).0 @ v, x.1 @ 22}`. Note that the first match-pair is now
/// simpler (and, in fact, irrefutable).
///
/// But there may also be candidates that the test just doesn't
/// apply to. For example, consider the case of #29740:
///
/// ```rust
/// match x {
/// "foo" => ...,
/// "bar" => ...,
/// "baz" => ...,
/// _ => ...,
/// }
/// ```
///
/// Here the match-pair we are testing will be `x @ "foo"`, and we
/// will generate an `Eq` test. Because `"bar"` and `"baz"` are different
/// constants, we will decide that these later candidates are just not
/// informed by the eq test. So we'll wind up with three candidate sets:
///
/// - If outcome is that `x == "foo"` (one candidate, derived from `x @ "foo"`)
/// - If outcome is that `x != "foo"` (empty list of candidates)
/// - Otherwise (three candidates, `x @ "bar"`, `x @ "baz"`, `x @
/// _`). Here we have the invariant that everything in the
/// otherwise list is of **lower priority** than the stuff in the
/// other lists.
///
/// So we'll compile the test. For each outcome of the test, we
/// recursively call `match_candidates` with the corresponding set
/// of candidates. But note that this set is now inexhaustive: for
/// example, in the case where the test returns false, there are
/// NO candidates, even though there is stll a value to be
/// matched. So we'll collect the return values from
/// `match_candidates`, which are the blocks where control-flow
/// goes if none of the candidates matched. At this point, we can
/// continue with the "otherwise" list.
///
/// If you apply this to the above test, you basically wind up
/// with an if-else-if chain, testing each candidate in turn,
/// which is precisely what we want.
fn test_candidates<'pat>(&mut self,
span: Span,
arm_blocks: &mut ArmBlocks,
candidates: &[Candidate<'pat, 'tcx>],
block: BasicBlock)
-> (Vec<BasicBlock>, usize)
{
// extract the match-pair from the highest priority candidate
let match_pair = &candidates.last().unwrap().match_pairs[0];
let mut test = self.test(match_pair);

Expand All @@ -331,35 +470,57 @@ impl<'a,'tcx> Builder<'a,'tcx> {
// available
match test.kind {
TestKind::SwitchInt { switch_ty, ref mut options, ref mut indices } => {
for candidate in &candidates {
self.add_cases_to_switch(&match_pair.lvalue,
candidate,
switch_ty,
options,
indices);
for candidate in candidates.iter().rev() {
if !self.add_cases_to_switch(&match_pair.lvalue,
candidate,
switch_ty,
options,
indices) {
break;
}
}
}
_ => { }
}

// perform the test, branching to one of N blocks. For each of
// those N possible outcomes, create a (initially empty)
// vector of candidates. Those are the candidates that still
// apply if the test has that particular outcome.
debug!("match_candidates: test={:?} match_pair={:?}", test, match_pair);
let target_blocks = self.perform_test(block, &match_pair.lvalue, &test);

let mut target_candidates: Vec<_> = (0..target_blocks.len()).map(|_| vec![]).collect();

for candidate in &candidates {
self.sort_candidate(&match_pair.lvalue,
&test,
candidate,
&mut target_candidates);
}

for (target_block, target_candidates) in
// Sort the candidates into the appropriate vector in
// `target_candidates`. Note that at some point we may
// encounter a candidate where the test is not relevant; at
// that point, we stop sorting.
let tested_candidates =
candidates.iter()
.rev()
.take_while(|c| self.sort_candidate(&match_pair.lvalue,
&test,
c,
&mut target_candidates))
.count();
assert!(tested_candidates > 0); // at least the last candidate ought to be tested

// For each outcome of test, process the candidates that still
// apply. Collect a list of blocks where control flow will
// branch if one of the `target_candidate` sets is not
// exhaustive.
let otherwise: Vec<_> =
target_blocks.into_iter()
.zip(target_candidates.into_iter())
{
self.match_candidates(span, arm_blocks, target_candidates, target_block);
}
.zip(target_candidates)
.flat_map(|(target_block, target_candidates)| {
self.match_candidates(span,
arm_blocks,
target_candidates,
target_block)
})
.collect();

(otherwise, tested_candidates)
}

/// Initializes each of the bindings from the candidate by
Expand Down
58 changes: 33 additions & 25 deletions src/librustc_mir/build/matches/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,11 @@ impl<'a,'tcx> Builder<'a,'tcx> {
switch_ty: Ty<'tcx>,
options: &mut Vec<ConstVal>,
indices: &mut FnvHashMap<ConstVal, usize>)
-> bool
{
let match_pair = match candidate.match_pairs.iter().find(|mp| mp.lvalue == *test_lvalue) {
Some(match_pair) => match_pair,
_ => { return; }
_ => { return false; }
};

match *match_pair.pattern.kind {
Expand All @@ -121,11 +122,10 @@ impl<'a,'tcx> Builder<'a,'tcx> {
options.push(value.clone());
options.len() - 1
});
true
}

PatternKind::Range { .. } => {
}

PatternKind::Range { .. } |
PatternKind::Constant { .. } |
PatternKind::Variant { .. } |
PatternKind::Slice { .. } |
Expand All @@ -134,6 +134,8 @@ impl<'a,'tcx> Builder<'a,'tcx> {
PatternKind::Binding { .. } |
PatternKind::Leaf { .. } |
PatternKind::Deref { .. } => {
// don't know how to add these patterns to a switch
false
}
}
}
Expand Down Expand Up @@ -284,18 +286,29 @@ impl<'a,'tcx> Builder<'a,'tcx> {
/// P0` to the `resulting_candidates` entry corresponding to the
/// variant `Some`.
///
/// In many cases we will add the `candidate` to more than one
/// outcome. For example, say that the test is `x == 22`, but the
/// candidate is `x @ 13..55`. In that case, if the test is true,
/// then we know that the candidate applies (without this match
/// pair, potentially, though we don't optimize this due to
/// #29623). If the test is false, the candidate may also apply
/// (with the match pair, still).
/// However, in some cases, the test may just not be relevant to
/// candidate. For example, suppose we are testing whether `foo.x == 22`,
/// but in one match arm we have `Foo { x: _, ... }`... in that case,
/// the test for what value `x` has has no particular relevance
/// to this candidate. In such cases, this function just returns false
/// without doing anything. This is used by the overall `match_candidates`
/// algorithm to structure the match as a whole. See `match_candidates` for
/// more details.
///
/// FIXME(#29623). In some cases, we have some tricky choices to
/// make. for example, if we are testing that `x == 22`, but the
/// candidate is `x @ 13..55`, what should we do? In the event
/// that the test is true, we know that the candidate applies, but
/// in the event of false, we don't know 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.
pub fn sort_candidate<'pat>(&mut self,
test_lvalue: &Lvalue<'tcx>,
test: &Test<'tcx>,
candidate: &Candidate<'pat, 'tcx>,
resulting_candidates: &mut [Vec<Candidate<'pat, 'tcx>>]) {
resulting_candidates: &mut [Vec<Candidate<'pat, 'tcx>>])
-> bool {
// Find the match_pair for this lvalue (if any). At present,
// afaik, there can be at most one. (In the future, if we
// adopted a more general `@` operator, there might be more
Expand All @@ -311,7 +324,7 @@ impl<'a,'tcx> Builder<'a,'tcx> {
None => {
// We are not testing this lvalue. Therefore, this
// candidate applies to ALL outcomes.
return self.add_to_all_candidate_sets(candidate, resulting_candidates);
return false;
}
};

Expand All @@ -329,9 +342,10 @@ impl<'a,'tcx> Builder<'a,'tcx> {
subpatterns,
candidate);
resulting_candidates[variant_index].push(new_candidate);
true
}
_ => {
self.add_to_all_candidate_sets(candidate, resulting_candidates);
false
}
}
}
Expand All @@ -349,9 +363,10 @@ impl<'a,'tcx> Builder<'a,'tcx> {
let new_candidate = self.candidate_without_match_pair(match_pair_index,
candidate);
resulting_candidates[index].push(new_candidate);
true
}
_ => {
self.add_to_all_candidate_sets(candidate, resulting_candidates);
false
}
}
}
Expand All @@ -367,8 +382,9 @@ impl<'a,'tcx> Builder<'a,'tcx> {
let new_candidate = self.candidate_without_match_pair(match_pair_index,
candidate);
resulting_candidates[0].push(new_candidate);
true
} else {
self.add_to_all_candidate_sets(candidate, resulting_candidates);
false
}
}
}
Expand All @@ -392,14 +408,6 @@ impl<'a,'tcx> Builder<'a,'tcx> {
}
}

fn add_to_all_candidate_sets<'pat>(&mut self,
candidate: &Candidate<'pat, 'tcx>,
resulting_candidates: &mut [Vec<Candidate<'pat, 'tcx>>]) {
for resulting_candidate in resulting_candidates {
resulting_candidate.push(candidate.clone());
}
}

fn candidate_after_variant_switch<'pat>(&mut self,
match_pair_index: usize,
adt_def: ty::AdtDef<'tcx>,
Expand Down Expand Up @@ -447,5 +455,5 @@ impl<'a,'tcx> Builder<'a,'tcx> {
}

fn is_switch_ty<'tcx>(ty: Ty<'tcx>) -> bool {
ty.is_integral() || ty.is_char()
ty.is_integral() || ty.is_char() || ty.is_bool()
}
Loading