auto merge of #8544 : dim-an/rust/fix-match-pipes, r=pcwalton

Pointers to bound variables shouldn't be stored before checking pattern,
otherwise piped patterns can conflict with each other (issue #6338).

Closes #6338.

2 files changed, 94 insertions, 20 deletions

diff --git a/src/librustc/middle/trans/_match.rs b/src/librustc/middle/trans/_match.rs
index bb1e3fa1718..1eeafeacc6f 100644
--- a/src/librustc/middle/trans/_match.rs
+++ b/src/librustc/middle/trans/_match.rs
@@ -399,10 +399,17 @@ struct ArmData<'self> {
     bindings_map: @BindingsMap
 }
 
+/**
+ * Info about Match.
+ * If all `pats` are matched then arm `data` will be executed.
+ * As we proceed `bound_ptrs` are filled with pointers to values to be bound,
+ * these pointers are stored in llmatch variables just before executing `data` arm.
+ */
 #[deriving(Clone)]
 struct Match<'self> {
     pats: ~[@ast::pat],
-    data: ArmData<'self>
+    data: ArmData<'self>,
+    bound_ptrs: ~[(ident, ValueRef)]
 }
 
 impl<'self> Repr for Match<'self> {
@@ -447,14 +454,13 @@ fn expand_nested_bindings<'r>(bcx: @mut Block,
                                 br.pats.slice(col + 1u,
                                            br.pats.len())));
 
-                let binding_info =
-                    br.data.bindings_map.get(&path_to_ident(path));
-
-                Store(bcx, val, binding_info.llmatch);
-                Match {
+                let mut res = Match {
                     pats: pats,
-                    data: br.data.clone()
-                }
+                    data: br.data.clone(),
+                    bound_ptrs: br.bound_ptrs.clone()
+                };
+                res.bound_ptrs.push((path_to_ident(path), val));
+                res
             }
             _ => (*br).clone(),
         }
@@ -496,13 +502,11 @@ fn enter_match<'r>(bcx: @mut Block,
                         br.pats.slice(col + 1u, br.pats.len()));
 
                 let this = br.pats[col];
+                let mut bound_ptrs = br.bound_ptrs.clone();
                 match this.node {
                     ast::pat_ident(_, ref path, None) => {
                         if pat_is_binding(dm, this) {
-                            let binding_info =
-                                br.data.bindings_map.get(
-                                    &path_to_ident(path));
-                            Store(bcx, val, binding_info.llmatch);
+                            bound_ptrs.push((path_to_ident(path), val));
                         }
                     }
                     _ => {}
@@ -510,7 +514,8 @@ fn enter_match<'r>(bcx: @mut Block,
 
                 result.push(Match {
                     pats: pats,
-                    data: br.data.clone()
+                    data: br.data.clone(),
+                    bound_ptrs: bound_ptrs
                 });
             }
             None => ()
@@ -1294,7 +1299,6 @@ fn store_non_ref_bindings(bcx: @mut Block,
 
 fn insert_lllocals(bcx: @mut Block,
                    bindings_map: &BindingsMap,
-                   binding_mode: IrrefutablePatternBindingMode,
                    add_cleans: bool) -> @mut Block {
     /*!
      * For each binding in `data.bindings_map`, adds an appropriate entry into
@@ -1302,10 +1306,7 @@ fn insert_lllocals(bcx: @mut Block,
      * the bindings.
      */
 
-    let llmap = match binding_mode {
-        BindLocal => bcx.fcx.lllocals,
-        BindArgument => bcx.fcx.llargs
-    };
+    let llmap = bcx.fcx.lllocals;
 
     for (&ident, &binding_info) in bindings_map.iter() {
         let llval = match binding_info.trmode {
@@ -1358,7 +1359,7 @@ fn compile_guard(bcx: @mut Block,
     bcx = store_non_ref_bindings(bcx,
                                  data.bindings_map,
                                  Some(&mut temp_cleanups));
-    bcx = insert_lllocals(bcx, data.bindings_map, BindLocal, false);
+    bcx = insert_lllocals(bcx, data.bindings_map, false);
 
     let val = unpack_result!(bcx, {
         do with_scope_result(bcx, guard_expr.info(),
@@ -1418,6 +1419,10 @@ fn compile_submatch(bcx: @mut Block,
     }
     if m[0].pats.len() == 0u {
         let data = &m[0].data;
+        for &(ref ident, ref value_ptr) in m[0].bound_ptrs.iter() {
+            let llmatch = data.bindings_map.get(ident).llmatch;
+            Store(bcx, *value_ptr, llmatch);
+        }
         match data.arm.guard {
             Some(guard_expr) => {
                 bcx = compile_guard(bcx,
@@ -1843,6 +1848,7 @@ fn trans_match_inner(scope_cx: @mut Block,
             matches.push(Match {
                 pats: ~[*p],
                 data: arm_data.clone(),
+                bound_ptrs: ~[],
             });
         }
     }
@@ -1875,7 +1881,7 @@ fn trans_match_inner(scope_cx: @mut Block,
         }
 
         // insert bindings into the lllocals map and add cleanups
-        bcx = insert_lllocals(bcx, arm_data.bindings_map, BindLocal, true);
+        bcx = insert_lllocals(bcx, arm_data.bindings_map, true);
 
         bcx = controlflow::trans_block(bcx, &arm_data.arm.body, dest);
         bcx = trans_block_cleanups(bcx, block_cleanups(arm_data.bodycx));
diff --git a/src/test/run-pass/match-pipe-binding.rs b/src/test/run-pass/match-pipe-binding.rs
new file mode 100644
index 00000000000..b4933773c03
--- /dev/null
+++ b/src/test/run-pass/match-pipe-binding.rs
@@ -0,0 +1,68 @@
+// Copyright 2013 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.
+
+fn test1() {
+    // from issue 6338
+    match ((1, ~"a"), (2, ~"b")) {
+        ((1, a), (2, b)) | ((2, b), (1, a)) => {
+                assert_eq!(a, ~"a");
+                assert_eq!(b, ~"b");
+            },
+            _ => fail!(),
+    }
+}
+
+fn test2() {
+    match (1, 2, 3) {
+        (1, a, b) | (2, b, a) => {
+            assert_eq!(a, 2);
+            assert_eq!(b, 3);
+        },
+        _ => fail!(),
+    }
+}
+
+fn test3() {
+    match (1, 2, 3) {
+        (1, ref a, ref b) | (2, ref b, ref a) => {
+            assert_eq!(*a, 2);
+            assert_eq!(*b, 3);
+        },
+        _ => fail!(),
+    }
+}
+
+fn test4() {
+    match (1, 2, 3) {
+        (1, a, b) | (2, b, a) if a == 2 => {
+            assert_eq!(a, 2);
+            assert_eq!(b, 3);
+        },
+        _ => fail!(),
+    }
+}
+
+fn test5() {
+    match (1, 2, 3) {
+        (1, ref a, ref b) | (2, ref b, ref a) if *a == 2 => {
+            assert_eq!(*a, 2);
+            assert_eq!(*b, 3);
+        },
+        _ => fail!(),
+    }
+}
+
+fn main() {
+    test1();
+    test2();
+    test3();
+    test4();
+    test5();
+}