librustc: Forbid pattern bindings after @s, for memory safety.

src/doc/rust.md

@@ -3309,7 +3309,12 @@ enum List { Nil, Cons(uint, Box<List>) }
 fn is_sorted(list: &List) -> bool {
     match *list {
         Nil | Cons(_, box Nil) => true,
-        Cons(x, ref r @ box Cons(y, _)) => (x <= y) && is_sorted(&**r)
+        Cons(x, ref r @ box Cons(_, _)) => {
+            match *r {
+                box Cons(y, _) => (x <= y) && is_sorted(&**r),
+                _ => fail!()
+            }
+        }
     }
 }
 

src/libcollections/vec.rs

@@ -1724,7 +1724,7 @@ mod tests {
             }
         }
 
-        let mut count_x @ mut count_y = 0;
+        let (mut count_x, mut count_y) = (0, 0);
         {
             let mut tv = TwoVec {
                 x: Vec::new(),

src/librustc/lint/context.rs

@@ -630,8 +630,9 @@ impl LintPass for GatherNodeLevels {
         match it.node {
             ast::ItemEnum(..) => {
                 let lint_id = LintId::of(builtin::VARIANT_SIZE_DIFFERENCE);
-                match cx.lints.get_level_source(lint_id) {
-                    lvlsrc @ (lvl, _) if lvl != Allow => {
+                let lvlsrc = cx.lints.get_level_source(lint_id);
+                match lvlsrc {
+                    (lvl, _) if lvl != Allow => {
                         cx.node_levels.borrow_mut()
                             .insert((it.id, lint_id), lvlsrc);
                     },

src/librustc/middle/borrowck/gather_loans/restrictions.rs

@@ -139,27 +139,36 @@ impl<'a> RestrictionsContext<'a> {
                 Safe
             }
 
-            mc::cat_deref(cmt_base, _, pk @ mc::BorrowedPtr(ty::MutBorrow, lt)) |
-            mc::cat_deref(cmt_base, _, pk @ mc::Implicit(ty::MutBorrow, lt)) => {
-                // R-Deref-Mut-Borrowed
-                if !self.bccx.is_subregion_of(self.loan_region, lt) {
-                    self.bccx.report(
-                        BckError {
-                            span: self.span,
-                            cause: self.cause,
-                            cmt: cmt_base,
-                            code: err_borrowed_pointer_too_short(
-                                self.loan_region, lt)});
-                    return Safe;
+            mc::cat_deref(cmt_base, _, pk) => {
+                match pk {
+                    mc::BorrowedPtr(ty::MutBorrow, lt) |
+                    mc::Implicit(ty::MutBorrow, lt) => {
+                        // R-Deref-Mut-Borrowed
+                        if !self.bccx.is_subregion_of(self.loan_region, lt) {
+                            self.bccx.report(
+                                BckError {
+                                    span: self.span,
+                                    cause: self.cause,
+                                    cmt: cmt_base,
+                                    code: err_borrowed_pointer_too_short(
+                                        self.loan_region, lt)});
+                            return Safe;
+                        }
+
+                        let result = self.restrict(cmt_base);
+                        self.extend(result, cmt.mutbl, LpDeref(pk))
+                    }
+                    mc::UnsafePtr(..) => {
+                        // We are very trusting when working with unsafe
+                        // pointers.
+                        Safe
+                    }
+                    _ => {
+                        self.bccx.tcx.sess.span_bug(self.span,
+                                                    "unhandled memcat in \
+                                                     cat_deref")
+                    }
                 }
-
-                let result = self.restrict(cmt_base);
-                self.extend(result, cmt.mutbl, LpDeref(pk))
-            }
-
-            mc::cat_deref(_, _, mc::UnsafePtr(..)) => {
-                // We are very trusting when working with unsafe pointers.
-                Safe
             }
 
             mc::cat_discr(cmt_base, _) => {

src/librustc/middle/check_match.rs

@@ -145,6 +145,9 @@ fn check_expr(cx: &mut MatchCheckCtxt, ex: &Expr) {
                 check_legality_of_move_bindings(cx,
                                                 arm.guard.is_some(),
                                                 arm.pats.as_slice());
+                for pat in arm.pats.iter() {
+                    check_legality_of_bindings_in_at_patterns(cx, &**pat);
+                }
             }
 
             // Second, if there is a guard on each arm, make sure it isn't
@@ -200,6 +203,7 @@ fn check_expr(cx: &mut MatchCheckCtxt, ex: &Expr) {
 
             // Check legality of move bindings.
             check_legality_of_move_bindings(cx, false, [ *pat ]);
+            check_legality_of_bindings_in_at_patterns(cx, &**pat);
         }
         _ => ()
     }
@@ -455,8 +459,12 @@ fn all_constructors(cx: &MatchCheckCtxt, left_ty: ty::t,
 
 // Note: is_useful doesn't work on empty types, as the paper notes.
 // So it assumes that v is non-empty.
-fn is_useful(cx: &MatchCheckCtxt, matrix @ &Matrix(ref rows): &Matrix,
-             v: &[Gc<Pat>], witness: WitnessPreference) -> Usefulness {
+fn is_useful(cx: &MatchCheckCtxt,
+             matrix: &Matrix,
+             v: &[Gc<Pat>],
+             witness: WitnessPreference)
+             -> Usefulness {
+    let &Matrix(ref rows) = matrix;
     debug!("{:}", matrix);
     if rows.len() == 0u {
         return match witness {
@@ -819,8 +827,9 @@ fn check_local(cx: &mut MatchCheckCtxt, loc: &Local) {
         None => ()
     }
 
-    // Check legality of move bindings.
+    // Check legality of move bindings and `@` patterns.
     check_legality_of_move_bindings(cx, false, [ loc.pat ]);
+    check_legality_of_bindings_in_at_patterns(cx, &*loc.pat);
 }
 
 fn check_fn(cx: &mut MatchCheckCtxt,
@@ -840,6 +849,7 @@ fn check_fn(cx: &mut MatchCheckCtxt,
             None => ()
         }
         check_legality_of_move_bindings(cx, false, [input.pat]);
+        check_legality_of_bindings_in_at_patterns(cx, &*input.pat);
     }
 }
 
@@ -856,7 +866,6 @@ fn is_refutable(cx: &MatchCheckCtxt, pat: Gc<Pat>) -> Option<Gc<Pat>> {
 }
 
 // Legality of move bindings checking
-
 fn check_legality_of_move_bindings(cx: &MatchCheckCtxt,
                                    has_guard: bool,
                                    pats: &[Gc<Pat>]) {
@@ -966,3 +975,32 @@ impl<'a> Delegate for MutationChecker<'a> {
     }
 }
 
+/// Forbids bindings in `@` patterns. This is necessary for memory safety,
+/// because of the way rvalues are handled in the borrow check. (See issue
+/// #14587.)
+fn check_legality_of_bindings_in_at_patterns(cx: &MatchCheckCtxt, pat: &Pat) {
+    let mut visitor = AtBindingPatternVisitor {
+        cx: cx,
+    };
+    visitor.visit_pat(pat, true);
+}
+
+struct AtBindingPatternVisitor<'a,'b> {
+    cx: &'a MatchCheckCtxt<'b>,
+}
+
+impl<'a,'b> Visitor<bool> for AtBindingPatternVisitor<'a,'b> {
+    fn visit_pat(&mut self, pat: &Pat, bindings_allowed: bool) {
+        if !bindings_allowed && pat_is_binding(&self.cx.tcx.def_map, pat) {
+            self.cx.tcx.sess.span_err(pat.span,
+                                      "pattern bindings are not allowed \
+                                       after an `@`");
+        }
+
+        match pat.node {
+            PatIdent(_, _, Some(_)) => visit::walk_pat(self, pat, false),
+            _ => visit::walk_pat(self, pat, bindings_allowed),
+        }
+    }
+}
+

src/librustc/middle/typeck/infer/region_inference/mod.rs

@@ -600,8 +600,9 @@ impl<'a> RegionVarBindings<'a> {
                         b).as_slice());
           }
 
-          (f @ ReFree(ref fr), ReScope(s_id)) |
-          (ReScope(s_id), f @ ReFree(ref fr)) => {
+          (ReFree(ref fr), ReScope(s_id)) |
+          (ReScope(s_id), ReFree(ref fr)) => {
+            let f = ReFree(*fr);
             // A "free" region can be interpreted as "some region
             // at least as big as the block fr.scope_id".  So, we can
             // reasonably compare free regions and scopes:
@@ -706,8 +707,9 @@ impl<'a> RegionVarBindings<'a> {
                             b).as_slice());
             }
 
-            (ReFree(ref fr), s @ ReScope(s_id)) |
-            (s @ ReScope(s_id), ReFree(ref fr)) => {
+            (ReFree(ref fr), ReScope(s_id)) |
+            (ReScope(s_id), ReFree(ref fr)) => {
+                let s = ReScope(s_id);
                 // Free region is something "at least as big as
                 // `fr.scope_id`."  If we find that the scope `fr.scope_id` is bigger
                 // than the scope `s_id`, then we can say that the GLB

src/test/compile-fail/bind-by-move-no-sub-bindings.rs → src/test/compile-fail/pattern-bindings-after-at.rs

@@ -1,4 +1,4 @@
-// Copyright 2012 The Rust Project Developers. See the COPYRIGHT
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
 // file at the top-level directory of this distribution and at
 // http://rust-lang.org/COPYRIGHT.
 //
@@ -8,18 +8,19 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-struct X { x: (), }
-
-impl Drop for X {
-    fn drop(&mut self) {
-        println!("destructor runs");
-    }
+enum Option<T> {
+    None,
+    Some(T),
 }
 
 fn main() {
-    let x = Some(X { x: () });
-    match x {
-        Some(_y @ ref _z) => { }, //~ ERROR cannot bind by-move with sub-bindings
-        None => fail!()
+    match &mut Some(1i) {
+        ref mut z @ &Some(ref a) => {
+        //~^ ERROR pattern bindings are not allowed after an `@`
+            **z = None;
+            println!("{}", *a);
+        }
+        _ => ()
     }
 }
+

src/test/run-pass/match-pattern-bindings.rs

@@ -15,24 +15,15 @@ fn main() {
         ref b @ None => b
     }, &Some(1i));
     assert_eq!(match value {
-        ref a @ ref _c @ Some(_) => a,
-        ref b @ None => b
-    }, &Some(1i));
-    assert_eq!(match value {
-        _a @ ref c @ Some(_) => c,
+        ref c @ Some(_) => c,
         ref b @ None => b
     }, &Some(1i));
     assert_eq!(match "foobarbaz" {
-        _a @ b @ _ => b
+        b @ _ => b
     }, "foobarbaz");
-
-    let a @ b @ c = "foobarbaz";
+    let a @ _ = "foobarbaz";
     assert_eq!(a, "foobarbaz");
-    assert_eq!(b, "foobarbaz");
-    assert_eq!(c, "foobarbaz");
     let value = Some(true);
-    let ref a @ b @ ref c = value;
+    let ref a @ _ = value;
     assert_eq!(a, &Some(true));
-    assert_eq!(b, Some(true));
-    assert_eq!(c, &Some(true));
 }