]> git.lizzy.rs Git - rust.git/commitdiff
Adjust codegen logic for range and guarded arms
authorEdward Wang <edward.yu.wang@gmail.com>
Sun, 4 May 2014 22:16:16 +0000 (06:16 +0800)
committerEdward Wang <edward.yu.wang@gmail.com>
Mon, 5 May 2014 12:17:59 +0000 (20:17 +0800)
By carefully distinguishing falling back to the default arm from moving
on to the next pattern, this patch adjusts the codegen logic for range
and guarded arms of pattern matching expression. It is a more
appropriate way of fixing #12582 and #13027 without causing regressions
such as #13867.

Closes #13867

src/librustc/middle/trans/_match.rs
src/test/run-pass/issue-13027.rs
src/test/run-pass/issue-13867.rs [new file with mode: 0644]

index 1b61e2bfb08af8d42449dd5b5ec6e97caf6f4a4c..7206f3f03f5e6e3c6303d41e767818efe96d72b8 100644 (file)
@@ -526,7 +526,7 @@ fn enter_default<'a, 'b>(
     // Collect all of the matches that can match against anything.
     let matches = enter_match(bcx, dm, m, col, val, |p| {
         match p.node {
-          ast::PatWild | ast::PatWildMulti | ast::PatTup(_) => Some(Vec::new()),
+          ast::PatWild | ast::PatWildMulti => Some(Vec::new()),
           ast::PatIdent(_, _, None) if pat_is_binding(dm, p) => Some(Vec::new()),
           _ => None
         }
@@ -712,18 +712,7 @@ fn enter_opt<'a, 'b>(
             }
             _ => {
                 assert_is_binding_or_wild(bcx, p);
-                // In most cases, a binding/wildcard match be
-                // considered to match against any Opt. However, when
-                // doing vector pattern matching, submatches are
-                // considered even if the eventual match might be from
-                // a different submatch. Thus, when a submatch fails
-                // when doing a vector match, we proceed to the next
-                // submatch. Thus, including a default match would
-                // cause the default match to fire spuriously.
-                match *opt {
-                    vec_len(..) => None,
-                    _ => Some(Vec::from_elem(variant_size, dummy))
-                }
+                Some(Vec::from_elem(variant_size, dummy))
             }
         };
         i += 1;
@@ -1363,7 +1352,8 @@ fn compile_guard<'a, 'b>(
                  data: &ArmData,
                  m: &'a [Match<'a, 'b>],
                  vals: &[ValueRef],
-                 chk: &FailureHandler)
+                 chk: &FailureHandler,
+                 has_genuine_default: bool)
                  -> &'b Block<'b> {
     debug!("compile_guard(bcx={}, guard_expr={}, m={}, vals={})",
            bcx.to_str(),
@@ -1394,7 +1384,17 @@ fn compile_guard<'a, 'b>(
         // Guard does not match: free the values we copied,
         // and remove all bindings from the lllocals table
         let bcx = drop_bindings(bcx, data);
-        compile_submatch(bcx, m, vals, chk);
+        match chk {
+            // If the default arm is the only one left, move on to the next
+            // condition explicitly rather than (possibly) falling back to
+            // the default arm.
+            &JumpToBasicBlock(_) if m.len() == 1 && has_genuine_default => {
+                Br(bcx, chk.handle_fail());
+            }
+            _ => {
+                compile_submatch(bcx, m, vals, chk, has_genuine_default);
+            }
+        };
         bcx
     });
 
@@ -1418,7 +1418,8 @@ fn compile_submatch<'a, 'b>(
                     bcx: &'b Block<'b>,
                     m: &'a [Match<'a, 'b>],
                     vals: &[ValueRef],
-                    chk: &FailureHandler) {
+                    chk: &FailureHandler,
+                    has_genuine_default: bool) {
     debug!("compile_submatch(bcx={}, m={}, vals={})",
            bcx.to_str(),
            m.repr(bcx.tcx()),
@@ -1448,7 +1449,8 @@ fn compile_submatch<'a, 'b>(
                                     m[0].data,
                                     m.slice(1, m.len()),
                                     vals,
-                                    chk);
+                                    chk,
+                                    has_genuine_default);
             }
             _ => ()
         }
@@ -1466,9 +1468,10 @@ fn compile_submatch<'a, 'b>(
                                   vals,
                                   chk,
                                   col,
-                                  val)
+                                  val,
+                                  has_genuine_default)
     } else {
-        compile_submatch_continue(bcx, m, vals, chk, col, val)
+        compile_submatch_continue(bcx, m, vals, chk, col, val, has_genuine_default)
     }
 }
 
@@ -1478,7 +1481,8 @@ fn compile_submatch_continue<'a, 'b>(
                              vals: &[ValueRef],
                              chk: &FailureHandler,
                              col: uint,
-                             val: ValueRef) {
+                             val: ValueRef,
+                             has_genuine_default: bool) {
     let fcx = bcx.fcx;
     let tcx = bcx.tcx();
     let dm = &tcx.def_map;
@@ -1512,7 +1516,7 @@ fn compile_submatch_continue<'a, 'b>(
                                             rec_fields.as_slice(),
                                             val).as_slice(),
                         rec_vals.append(vals_left.as_slice()).as_slice(),
-                        chk);
+                        chk, has_genuine_default);
             });
             return;
         }
@@ -1537,7 +1541,7 @@ fn compile_submatch_continue<'a, 'b>(
                                    val,
                                    n_tup_elts).as_slice(),
                          tup_vals.append(vals_left.as_slice()).as_slice(),
-                         chk);
+                         chk, has_genuine_default);
         return;
     }
 
@@ -1562,7 +1566,7 @@ fn compile_submatch_continue<'a, 'b>(
                          enter_tuple_struct(bcx, dm, m, col, val,
                                             struct_element_count).as_slice(),
                          llstructvals.append(vals_left.as_slice()).as_slice(),
-                         chk);
+                         chk, has_genuine_default);
         return;
     }
 
@@ -1571,7 +1575,7 @@ fn compile_submatch_continue<'a, 'b>(
         compile_submatch(bcx,
                          enter_uniq(bcx, dm, m, col, val).as_slice(),
                          (vec!(llbox)).append(vals_left.as_slice()).as_slice(),
-                         chk);
+                         chk, has_genuine_default);
         return;
     }
 
@@ -1580,7 +1584,7 @@ fn compile_submatch_continue<'a, 'b>(
         compile_submatch(bcx,
                          enter_region(bcx, dm, m, col, val).as_slice(),
                          (vec!(loaded_val)).append(vals_left.as_slice()).as_slice(),
-                         chk);
+                         chk, has_genuine_default);
         return;
     }
 
@@ -1637,9 +1641,9 @@ fn compile_submatch_continue<'a, 'b>(
 
     // Compile subtrees for each option
     for (i, opt) in opts.iter().enumerate() {
-        // In some cases in vector pattern matching, we need to override
-        // the failure case so that instead of failing, it proceeds to
-        // try more matching. branch_chk, then, is the proper failure case
+        // In some cases of range and vector pattern matching, we need to
+        // override the failure case so that instead of failing, it proceeds
+        // to try more matching. branch_chk, then, is the proper failure case
         // for the current conditional branch.
         let mut branch_chk = None;
         let mut opt_cx = else_cx;
@@ -1689,6 +1693,16 @@ fn compile_submatch_continue<'a, 'b>(
                       }
                   };
                   bcx = fcx.new_temp_block("compare_next");
+
+                  // If none of the sub-cases match, and the current condition
+                  // is guarded or has multiple patterns, move on to the next
+                  // condition, if there is any, rather than falling back to
+                  // the default.
+                  let guarded = m[i].data.arm.guard.is_some();
+                  let multi_pats = m[i].pats.len() > 1;
+                  if i+1 < len && (guarded || multi_pats) {
+                      branch_chk = Some(JumpToBasicBlock(bcx.llbb));
+                  }
                   CondBr(after_cx, matches, opt_cx.llbb, bcx.llbb);
               }
               compare_vec_len => {
@@ -1726,8 +1740,10 @@ fn compile_submatch_continue<'a, 'b>(
                   bcx = fcx.new_temp_block("compare_vec_len_next");
 
                   // If none of these subcases match, move on to the
-                  // next condition.
-                  branch_chk = Some(JumpToBasicBlock(bcx.llbb));
+                  // next condition if there is any.
+                  if i+1 < len {
+                      branch_chk = Some(JumpToBasicBlock(bcx.llbb));
+                  }
                   CondBr(after_cx, matches, opt_cx.llbb, bcx.llbb);
               }
               _ => ()
@@ -1767,27 +1783,38 @@ fn compile_submatch_continue<'a, 'b>(
                 compile_submatch(opt_cx,
                                  opt_ms.as_slice(),
                                  opt_vals.as_slice(),
-                                 chk)
+                                 chk,
+                                 has_genuine_default)
             }
             Some(branch_chk) => {
                 compile_submatch(opt_cx,
                                  opt_ms.as_slice(),
                                  opt_vals.as_slice(),
-                                 &branch_chk)
+                                 &branch_chk,
+                                 has_genuine_default)
             }
         }
     }
 
     // Compile the fall-through case, if any
-    if !exhaustive {
+    if !exhaustive && kind != single {
         if kind == compare || kind == compare_vec_len {
             Br(bcx, else_cx.llbb);
         }
-        if kind != single {
-            compile_submatch(else_cx,
-                             defaults.as_slice(),
-                             vals_left.as_slice(),
-                             chk);
+        match chk {
+            // If there is only one default arm left, move on to the next
+            // condition explicitly rather than (eventually) falling back to
+            // the last default arm.
+            &JumpToBasicBlock(_) if defaults.len() == 1 && has_genuine_default => {
+                Br(else_cx, chk.handle_fail());
+            }
+            _ => {
+                compile_submatch(else_cx,
+                                 defaults.as_slice(),
+                                 vals_left.as_slice(),
+                                 chk,
+                                 has_genuine_default);
+            }
         }
     }
 }
@@ -1892,7 +1919,22 @@ fn trans_match_inner<'a>(scope_cx: &'a Block<'a>,
         }));
     }
 
-    compile_submatch(bcx, matches.as_slice(), [discr_datum.val], &chk);
+    // `compile_submatch` works one column of arm patterns a time and
+    // then peels that column off. So as we progress, it may become
+    // impossible to know whether we have a genuine default arm, i.e.
+    // `_ => foo` or not. Sometimes it is important to know that in order
+    // to decide whether moving on to the next condition or falling back
+    // to the default arm.
+    let has_default = arms.len() > 0 && {
+        let ref pats = arms.last().unwrap().pats;
+
+        pats.len() == 1
+        && match pats.last().unwrap().node {
+            ast::PatWild => true, _ => false
+        }
+    };
+
+    compile_submatch(bcx, matches.as_slice(), [discr_datum.val], &chk, has_default);
 
     let mut arm_cxs = Vec::new();
     for arm_data in arm_datas.iter() {
index f2f0418a33fbab7f591b469510325da00caffec2..fc3b1c7cb1ab24d620c683c8c42cbc8cd53aad16 100644 (file)
@@ -23,6 +23,7 @@ pub fn main() {
     multi_pats_shadow_range();
     lit_shadow_multi_pats();
     range_shadow_multi_pats();
+    misc();
 }
 
 fn lit_shadow_range() {
@@ -168,3 +169,18 @@ fn range_shadow_multi_pats() {
         _ => 3,
     });
 }
+
+fn misc() {
+    enum Foo {
+        Bar(uint, bool)
+    }
+    // This test basically mimics how trace_macros! macro is implemented,
+    // which is a rare combination of vector patterns, multiple wild-card
+    // patterns and guard functions.
+    let r = match [Bar(0, false)].as_slice() {
+        [Bar(_, pred)] if pred => 1,
+        [Bar(_, pred)] if !pred => 2,
+        _ => 0,
+    };
+    assert_eq!(2, r);
+}
diff --git a/src/test/run-pass/issue-13867.rs b/src/test/run-pass/issue-13867.rs
new file mode 100644 (file)
index 0000000..fb76dbf
--- /dev/null
@@ -0,0 +1,57 @@
+// Copyright 2012-2014 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.
+
+// Test that codegen works correctly when there are multiple refutable
+// patterns in match expression.
+
+enum Foo {
+    FooUint(uint),
+    FooNullary,
+}
+
+fn main() {
+    let r = match (FooNullary, 'a') {
+        (FooUint(..), 'a'..'z') => 1,
+        (FooNullary, 'x') => 2,
+        _ => 0
+    };
+    assert_eq!(r, 0);
+
+    let r = match (FooUint(0), 'a') {
+        (FooUint(1), 'a'..'z') => 1,
+        (FooUint(..), 'x') => 2,
+        (FooNullary, 'a') => 3,
+        _ => 0
+    };
+    assert_eq!(r, 0);
+
+    let r = match ('a', FooUint(0)) {
+        ('a'..'z', FooUint(1)) => 1,
+        ('x', FooUint(..)) => 2,
+        ('a', FooNullary) => 3,
+        _ => 0
+    };
+    assert_eq!(r, 0);
+
+    let r = match ('a', 'a') {
+        ('a'..'z', 'b') => 1,
+        ('x', 'a'..'z') => 2,
+        _ => 0
+    };
+    assert_eq!(r, 0);
+
+    let r = match ('a', 'a') {
+        ('a'..'z', 'b') => 1,
+        ('x', 'a'..'z') => 2,
+        ('a', 'a') => 3,
+        _ => 0
+    };
+    assert_eq!(r, 3);
+}