Reorganize match construction to be O(n) instead of O(n^2). Whereas

before we iterated over the test and each outcome thereof, and then
checked processed every candidate against this outcome, we now organize
the walk differently. Instead, we visit each candidate and say "Here is
the test being performed. Figure out the resulting candidates for each
possible outcome and add yourself into the appropriate places."

Author: nikomatsakis

src/librustc_mir/build/matches/mod.rs

@@ -345,17 +345,20 @@ impl<'a,'tcx> Builder<'a,'tcx> {
         debug!("match_candidates: test={:?} match_pair={:?}", test, match_pair);
         let target_blocks = self.perform_test(block, &match_pair.lvalue, &test);
 
-        for (outcome, target_block) in target_blocks.into_iter().enumerate() {
-            let applicable_candidates: Vec<_> =
-                candidates.iter()
-                          .filter_map(|candidate| {
-                              self.candidate_under_assumption(&match_pair.lvalue,
-                                                              &test.kind,
-                                                              outcome,
-                                                              candidate)
-                          })
-                          .collect();
-            self.match_candidates(span, arm_blocks, applicable_candidates, target_block);
+        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
+            target_blocks.into_iter()
+                         .zip(target_candidates.into_iter())
+        {
+            self.match_candidates(span, arm_blocks, target_candidates, target_block);
         }
     }
 

src/librustc_mir/build/matches/test.rs

@@ -21,7 +21,7 @@ use hair::*;
 use repr::*;
 use rustc_data_structures::fnv::FnvHashMap;
 use rustc::middle::const_eval::ConstVal;
-use rustc::middle::ty::Ty;
+use rustc::middle::ty::{self, Ty};
 use syntax::codemap::Span;
 
 impl<'a,'tcx> Builder<'a,'tcx> {
@@ -271,248 +271,172 @@ impl<'a,'tcx> Builder<'a,'tcx> {
         target_blocks
     }
 
-    /// Given a candidate and the outcome of a test we have performed,
-    /// transforms the candidate into a new candidate that reflects
-    /// further tests still needed. Returns `None` if this candidate
-    /// has now been ruled out.
+    /// Given that we are performing `test` against `test_lvalue`,
+    /// this job sorts out what the status of `candidate` will be
+    /// after the test. The `resulting_candidates` vector stores, for
+    /// each possible outcome of `test`, a vector of the candidates
+    /// that will result. This fn should add a (possibly modified)
+    /// clone of candidate into `resulting_candidates` wherever
+    /// appropriate.
     ///
-    /// For example, if a candidate included the patterns `[x.0 @
-    /// Ok(P1), x.1 @ 22]`, and we did a switch test on `x.0` and
-    /// found the variant `Err` (as indicated by the `test_outcome`
-    /// parameter), we would return `None`. But if the test_outcome
-    /// were `Ok`, we would return `Some([x.0.downcast<Ok>.0 @ P1, x.1
-    /// @ 22])`.
-    pub fn candidate_under_assumption<'pat>(&mut self,
-                                            test_lvalue: &Lvalue<'tcx>,
-                                            test_kind: &TestKind<'tcx>,
-                                            test_outcome: usize,
-                                            candidate: &Candidate<'pat, 'tcx>)
-                                            -> Option<Candidate<'pat, 'tcx>> {
-        let result = self.match_pairs_under_assumption(test_lvalue,
-                                                       test_kind,
-                                                       test_outcome,
-                                                       &candidate.match_pairs);
-        match result {
-            Some(match_pairs) => Some(Candidate { match_pairs: match_pairs,
-                                                  bindings: candidate.bindings.clone(),
-                                                  guard: candidate.guard.clone(),
-                                                  arm_index: candidate.arm_index }),
-            None => None,
-        }
-    }
-
-    /// Helper for candidate_under_assumption that does the actual
-    /// work of transforming the list of match pairs.
-    fn match_pairs_under_assumption<'pat>(&mut self,
-                                          test_lvalue: &Lvalue<'tcx>,
-                                          test_kind: &TestKind<'tcx>,
-                                          test_outcome: usize,
-                                          match_pairs: &[MatchPair<'pat, 'tcx>])
-                                          -> Option<Vec<MatchPair<'pat, 'tcx>>> {
-        let mut result = vec![];
-
-        for match_pair in match_pairs {
-            // If the match pair is either:
-            // (1) testing a different lvalue; or,
-            // (2) the test doesn't tell us anything about this match-pair,
-            // then we have to retain it as for after the test is complete.
-            if
-                match_pair.lvalue != *test_lvalue || // (1)
-                !self.test_informs_match_pair(match_pair, test_kind, test_outcome) // (2)
-            {
-                result.push(match_pair.clone());
-                continue;
-            }
-
-            // otherwise, build up the consequence match pairs
-            let opt_consequent_match_pairs =
-                self.consequent_match_pairs_under_assumption(match_pair,
-                                                             test_kind,
-                                                             test_outcome);
-            match opt_consequent_match_pairs {
-                None => {
-                    // Right kind of test, but wrong outcome. That
-                    // means this **entire candidate** is
-                    // inapplicable, since the candidate is only
-                    // applicable if all of its match-pairs apply (and
-                    // this one doesn't).
-                    return None;
-                }
-
-                Some(consequent_match_pairs) => {
-                    // Test passed; add any new patterns we have to test to the final result.
-                    result.extend(consequent_match_pairs)
-                }
+    /// So, for example, if this candidate is `x @ Some(P0)` and the
+    /// test is a variant test, then we would add `(x as Option).0 @
+    /// 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).
+    pub fn sort_candidate<'pat>(&mut self,
+                                test_lvalue: &Lvalue<'tcx>,
+                                test: &Test<'tcx>,
+                                candidate: &Candidate<'pat, 'tcx>,
+                                resulting_candidates: &mut Vec<Vec<Candidate<'pat, 'tcx>>>) {
+        // 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
+        // than one, but it'd be very unusual to have two sides that
+        // both require tests; you'd expect one side to be simplified
+        // away.)
+        let tested_match_pair = candidate.match_pairs.iter()
+                                                     .enumerate()
+                                                     .filter(|&(_, mp)| mp.lvalue == *test_lvalue)
+                                                     .next();
+        let (match_pair_index, match_pair) = match tested_match_pair {
+            Some(pair) => pair,
+            None => {
+                // We are not testing this lvalue. Therefore, this
+                // candidate applies to ALL outcomes.
+                return self.add_to_all_candidate_sets(candidate, resulting_candidates);
             }
-        }
-
-        Some(result)
-    }
+        };
 
-    /// Given that we executed `test` to `match_pair.lvalue` with
-    /// outcome `test_outcome`, does that tell us anything about
-    /// whether `match_pair` applies?
-    ///
-    /// Often it does not. For example, if we are testing whether
-    /// the discriminant equals 4, and we find that it does not,
-    /// but the `match_pair` is testing if the discriminant equals 5,
-    /// that does not help us.
-    fn test_informs_match_pair<'pat>(&mut self,
-                                     match_pair: &MatchPair<'pat, 'tcx>,
-                                     test_kind: &TestKind<'tcx>,
-                                     _test_outcome: usize)
-                                     -> bool {
-        match *match_pair.pattern.kind {
-            PatternKind::Variant { .. } => {
-                match *test_kind {
-                    TestKind::Switch { .. } => true,
-                    _ => false,
+        match test.kind {
+            // If we are performing a variant switch, then this
+            // informs variant patterns, but nothing else.
+            TestKind::Switch { adt_def: tested_adt_def } => {
+                match *match_pair.pattern.kind {
+                    PatternKind::Variant { adt_def, variant_index, ref subpatterns } => {
+                        assert_eq!(adt_def, tested_adt_def);
+                        let new_candidate =
+                            self.candidate_after_variant_switch(match_pair_index,
+                                                                adt_def,
+                                                                variant_index,
+                                                                subpatterns,
+                                                                candidate);
+                        resulting_candidates[variant_index].push(new_candidate);
+                    }
+                    _ => {
+                        self.add_to_all_candidate_sets(candidate, resulting_candidates);
+                    }
                 }
             }
 
-            PatternKind::Constant { value: Literal::Value { .. } }
-            if is_switch_ty(match_pair.pattern.ty) => {
-                match *test_kind {
-                    TestKind::SwitchInt { .. } => true,
-
-                    // Did not do an integer equality test (which is always a SwitchInt).
-                    // So we learned nothing relevant to this match-pair.
-                    //
-                    // FIXME(#29623) we could use TestKind::Range to rule
-                    // things out here, in some cases.
-                    _ => false,
+            // If we are performing a switch over integers, then this informs integer
+            // equality, but nothing else.
+            //
+            // FIXME(#29623) we could use TestKind::Range to rule
+            // things out here, in some cases.
+            TestKind::SwitchInt { switch_ty: _, options: _, ref indices } => {
+                match *match_pair.pattern.kind {
+                    PatternKind::Constant { value: Literal::Value { ref value } }
+                    if is_switch_ty(match_pair.pattern.ty) => {
+                        let index = indices[value];
+                        let new_candidate = self.candidate_without_match_pair(match_pair_index,
+                                                                              candidate);
+                        resulting_candidates[index].push(new_candidate);
+                    }
+                    _ => {
+                        self.add_to_all_candidate_sets(candidate, resulting_candidates);
+                    }
                 }
             }
 
-            PatternKind::Constant { .. } |
-            PatternKind::Range { .. } |
-            PatternKind::Slice { .. } => {
+            TestKind::Eq { .. } |
+            TestKind::Range { .. } |
+            TestKind::Len { .. } => {
+                // These are all binary tests.
+                //
+                // FIXME(#29623) we can be more clever here
                 let pattern_test = self.test(&match_pair);
-                if pattern_test.kind == *test_kind {
-                    true
+                if pattern_test.kind == test.kind {
+                    let new_candidate = self.candidate_without_match_pair(match_pair_index,
+                                                                          candidate);
+                    resulting_candidates[0].push(new_candidate);
                 } else {
-                    // FIXME(#29623) in all 3 cases, we could sometimes do
-                    // better here. For example, if we are checking
-                    // whether the value is equal to X, and we find
-                    // that it is, that (may) imply value is not equal
-                    // to Y. Or, if the range tested is `3..5`, and
-                    // our range is `4..5`, then we know that our
-                    // range also does not apply. Similarly, if we
-                    // test that length is >= 5, and it fails, we also
-                    // know that length is not >= 7. etc.
-                    false
+                    self.add_to_all_candidate_sets(candidate, resulting_candidates);
                 }
             }
-
-            PatternKind::Array { .. } |
-            PatternKind::Wild |
-            PatternKind::Binding { .. } |
-            PatternKind::Leaf { .. } |
-            PatternKind::Deref { .. } => {
-                self.error_simplifyable(&match_pair)
-            }
         }
     }
 
-    /// Given that we executed `test` with outcome `test_outcome`,
-    /// what are the resulting match pairs? This can return either:
-    ///
-    /// - None, meaning that the test indicated that this outcome
-    ///   means that this match-pair is not the current one for the
-    ///   current discriminant (which rules out the enclosing
-    ///   candidate);
-    /// - Some(...), meaning that either the test didn't tell us whether this
-    ///   match-pair is correct or not, or that we DID match and now have
-    ///   subsequent matches to perform.
-    ///
-    /// As an example, consider:
-    ///
-    /// ```
-    /// match option {
-    ///     Ok(<pattern>) => ...,
-    ///     Err(_) => ...,
-    /// }
-    /// ```
-    ///
-    /// Suppose that the `test` is a `Switch` and the outcome is
-    /// `Ok`. Then in that case, the first arm will have a match-pair
-    /// of `option @ Ok(<pattern>)`. In that case we will return
-    /// `Some(vec![(option as Ok) @ <pattern>])`. The `Some` reuslt
-    /// indicates that the match-pair still applies, and we now have
-    /// to test `(option as Ok) @ <pattern>`.
-    ///
-    /// On the second arm, a `None` will be returned, because if we
-    /// observed that `option` has the discriminant `Ok`, then the
-    /// second arm cannot apply.
-    pub fn consequent_match_pairs_under_assumption<'pat>(&mut self,
-                                                         match_pair: &MatchPair<'pat, 'tcx>,
-                                                         test_kind: &TestKind<'tcx>,
-                                                         test_outcome: usize)
-                                                         -> Option<Vec<MatchPair<'pat, 'tcx>>> {
-        match *match_pair.pattern.kind {
-            PatternKind::Variant { adt_def, variant_index, ref subpatterns } => {
-                assert!(match *test_kind { TestKind::Switch { .. } => true,
-                                           _ => false });
-
-                if test_outcome != variant_index {
-                    return None; // Tested, but found the wrong variant.
-                }
+    fn candidate_without_match_pair<'pat>(&mut self,
+                                          match_pair_index: usize,
+                                          candidate: &Candidate<'pat, 'tcx>)
+                                          -> Candidate<'pat, 'tcx> {
+        let other_match_pairs =
+            candidate.match_pairs.iter()
+                                 .enumerate()
+                                 .filter(|&(index, _)| index != match_pair_index)
+                                 .map(|(_, mp)| mp.clone())
+                                 .collect();
+        Candidate {
+            match_pairs: other_match_pairs,
+            bindings: candidate.bindings.clone(),
+            guard: candidate.guard.clone(),
+            arm_index: candidate.arm_index,
+        }
+    }
 
-                // Correct variant. Extract the subitems and match
-                // those. The lvalue goes gets downcast, so
-                // e.g. `foo.bar` becomes `foo.bar as Variant`.
-                let elem = ProjectionElem::Downcast(adt_def, variant_index);
-                let downcast_lvalue = match_pair.lvalue.clone().elem(elem);
-                let consequent_match_pairs =
-                    subpatterns.iter()
-                               .map(|subpattern| {
-                                   let lvalue =
-                                       downcast_lvalue.clone().field(
-                                           subpattern.field);
-                                   MatchPair::new(lvalue, &subpattern.pattern)
-                               })
-                               .collect();
-                Some(consequent_match_pairs)
-            }
+    fn add_to_all_candidate_sets<'pat>(&mut self,
+                                       candidate: &Candidate<'pat, 'tcx>,
+                                       resulting_candidates: &mut Vec<Vec<Candidate<'pat, 'tcx>>>) {
+        for resulting_candidate in resulting_candidates {
+            resulting_candidate.push(candidate.clone());
+        }
+    }
 
-            PatternKind::Constant { value: Literal::Value { ref value } }
-            if is_switch_ty(match_pair.pattern.ty) => {
-                match *test_kind {
-                    TestKind::SwitchInt { switch_ty: _, options: _, ref indices } => {
-                        let index = indices[value];
-                        if index == test_outcome {
-                            Some(vec![]) // this value, nothing left to test
-                        } else {
-                            None // some other value, candidate is inapplicable
-                        }
-                    }
+    fn candidate_after_variant_switch<'pat>(&mut self,
+                                            match_pair_index: usize,
+                                            adt_def: ty::AdtDef<'tcx>,
+                                            variant_index: usize,
+                                            subpatterns: &'pat [FieldPattern<'tcx>],
+                                            candidate: &Candidate<'pat, 'tcx>)
+                                            -> Candidate<'pat, 'tcx> {
+        let match_pair = &candidate.match_pairs[match_pair_index];
+
+        // So, if we have a match-pattern like `x @ Enum::Variant(P1, P2)`,
+        // we want to create a set of derived match-patterns like
+        // `(x as Variant).0 @ P1` and `(x as Variant).1 @ P1`.
+        let elem = ProjectionElem::Downcast(adt_def, variant_index);
+        let downcast_lvalue = match_pair.lvalue.clone().elem(elem); // `(x as Variant)`
+        let consequent_match_pairs =
+            subpatterns.iter()
+                       .map(|subpattern| {
+                           // e.g., `(x as Variant).0`
+                           let lvalue = downcast_lvalue.clone().field(subpattern.field);
+                           // e.g., `(x as Variant).0 @ P1`
+                           MatchPair::new(lvalue, &subpattern.pattern)
+                       });
 
-                    _ => {
-                        self.hir.span_bug(
-                            match_pair.pattern.span,
-                            &format!("did a switch-int, but value {:?} not found in cases",
-                                     value));
-                    }
-                }
-            }
+        // In addition, we need all the other match pairs from the old candidate.
+        let other_match_pairs =
+            candidate.match_pairs.iter()
+                                 .enumerate()
+                                 .filter(|&(index, _)| index != match_pair_index)
+                                 .map(|(_, mp)| mp.clone());
 
-            PatternKind::Constant { .. } |
-            PatternKind::Range { .. } |
-            PatternKind::Slice { .. } => {
-                if test_outcome == 0 {
-                    Some(vec![])
-                } else {
-                    None
-                }
-            }
+        let all_match_pairs = consequent_match_pairs.chain(other_match_pairs).collect();
 
-            PatternKind::Array { .. } |
-            PatternKind::Wild |
-            PatternKind::Binding { .. } |
-            PatternKind::Leaf { .. } |
-            PatternKind::Deref { .. } => {
-                self.error_simplifyable(match_pair)
-            }
+        Candidate {
+            match_pairs: all_match_pairs,
+            bindings: candidate.bindings.clone(),
+            guard: candidate.guard.clone(),
+            arm_index: candidate.arm_index,
         }
     }