Skip to content

[MIR] Make sure candidates are reversed before match_candidates. #30553

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

Merged
merged 3 commits into from
Jan 4, 2016
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
41 changes: 18 additions & 23 deletions src/librustc_mir/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,13 @@ impl<'a,'tcx> Builder<'a,'tcx> {
// assemble a list of candidates: there is one candidate per
// pattern, which means there may be more than one candidate
// *per arm*. These candidates are kept sorted such that the
// highest priority candidate comes last in the list. This the
// reverse of the order in which candidates are written in the
// source.
// highest priority candidate comes first in the list.
// (i.e. same order as in source)
let candidates: Vec<_> =
arms.iter()
.enumerate()
.rev() // highest priority comes last
.flat_map(|(arm_index, arm)| {
arm.patterns.iter()
.rev()
.map(move |pat| (arm_index, pat, arm.guard.clone()))
})
.map(|(arm_index, pattern, guard)| {
Expand Down Expand Up @@ -290,9 +287,9 @@ 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
/// candidates are sorted such that the first item in the list
/// has the 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
Expand Down Expand Up @@ -324,17 +321,17 @@ impl<'a,'tcx> Builder<'a,'tcx> {
unpack!(block = self.simplify_candidate(block, candidate));
}

// The candidates are inversely sorted by priority. Check to
// see whether the candidates in the front of the queue (and
// hence back of the vec) have satisfied all their match
// The candidates are sorted by priority. Check to see
// whether the higher priority candidates (and hence at
// the front of the vec) have satisfied all their match
// pairs.
let fully_matched =
candidates.iter().rev().take_while(|c| c.match_pairs.is_empty()).count();
candidates.iter().take_while(|c| c.match_pairs.is_empty()).count();
debug!("match_candidates: {:?} candidates fully matched", fully_matched);
for _ in 0..fully_matched {
let mut unmatched_candidates = candidates.split_off(fully_matched);
for candidate in candidates {
// If so, apply any bindings, test the guard (if any), and
// branch to the arm.
let candidate = candidates.pop().unwrap();
if let Some(b) = self.bind_and_guard_matched_candidate(block, arm_blocks, candidate) {
block = b;
} else {
Expand All @@ -346,13 +343,13 @@ impl<'a,'tcx> Builder<'a,'tcx> {

// 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() {
if unmatched_candidates.is_empty() {
return vec![block];
}

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

// If the target candidates were exhaustive, then we are done.
if otherwise.is_empty() {
Expand All @@ -361,15 +358,14 @@ impl<'a,'tcx> Builder<'a,'tcx> {

// 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 {
let untested_candidates = unmatched_candidates.split_off(tested_candidates);
if untested_candidates.len() == 0 {
return otherwise;
}

// 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)
self.match_candidates(span, arm_blocks, untested_candidates, join_block)
}

fn join_otherwise_blocks(&mut self,
Expand Down Expand Up @@ -461,7 +457,7 @@ impl<'a,'tcx> Builder<'a,'tcx> {
-> (Vec<BasicBlock>, usize)
{
// extract the match-pair from the highest priority candidate
let match_pair = &candidates.last().unwrap().match_pairs[0];
let match_pair = &candidates.first().unwrap().match_pairs[0];
let mut test = self.test(match_pair);

// most of the time, the test to perform is simply a function
Expand All @@ -470,7 +466,7 @@ impl<'a,'tcx> Builder<'a,'tcx> {
// available
match test.kind {
TestKind::SwitchInt { switch_ty, ref mut options, ref mut indices } => {
for candidate in candidates.iter().rev() {
for candidate in candidates.iter() {
if !self.add_cases_to_switch(&match_pair.lvalue,
candidate,
switch_ty,
Expand All @@ -497,7 +493,6 @@ impl<'a,'tcx> Builder<'a,'tcx> {
// that point, we stop sorting.
let tested_candidates =
candidates.iter()
.rev()
.take_while(|c| self.sort_candidate(&match_pair.lvalue,
&test,
c,
Expand Down
28 changes: 28 additions & 0 deletions src/test/run-pass/mir_match_arm_guard.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// #30527 - We were not generating arms with guards in certain cases.

#![feature(rustc_attrs)]

#[rustc_mir]
fn match_with_guard(x: Option<i8>) -> i8 {
match x {
Some(xyz) if xyz > 100 => 0,
Some(_) => -1,
None => -2
}
}

fn main() {
assert_eq!(match_with_guard(Some(111)), 0);
assert_eq!(match_with_guard(Some(2)), -1);
assert_eq!(match_with_guard(None), -2);
}