Rewrite match algorithm to avoid massive blowup in generated code for

large matches that fallback to Eq. When we encounter a case where the
test being performed does not inform the candidate at all, we just stop
testing the candidates at that point, rather than adding the candidate
to both outcomes. The former behavior was not WRONG, but it generated a
lot of code, whereas this new behavior degenerates to an if-else-if
tree.

Fixes #29740.

Author: nikomatsakis

src/librustc_mir/build/matches/mod.rs

@@ -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();
@@ -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);
@@ -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);
 
@@ -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

src/librustc_mir/build/matches/test.rs

@@ -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 {
@@ -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 { .. } |
@@ -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
             }
         }
     }
@@ -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
@@ -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;
             }
         };
 
@@ -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
                     }
                 }
             }
@@ -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
                     }
                 }
             }
@@ -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
                 }
             }
         }
@@ -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>,
@@ -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()
 }