first shot at integrating ref/value bindings into borrowck

(more needed)

Author: nikomatsakis

src/rustc/middle/borrowck.rs

@@ -454,6 +454,12 @@ impl methods of get_type_for_node for ty::ctxt {
     }
 }
 
+impl borrowck_ctxt {
+    fn is_subregion_of(r_sub: ty::region, r_sup: ty::region) -> bool {
+        region::is_subregion_of(self.tcx.region_map, r_sub, r_sup)
+    }
+}
+
 impl error_methods for borrowck_ctxt {
     fn report_if_err(bres: bckres<()>) {
         match bres {

src/rustc/middle/borrowck/gather_loans.rs

@@ -9,6 +9,7 @@
 import categorization::{public_methods, opt_deref_kind};
 import loan::public_methods;
 import preserve::{public_methods, preserve_condition, pc_ok, pc_if_pure};
+import ty::ty_region;
 
 export gather_loans;
 
@@ -105,10 +106,7 @@ fn req_loans_in_expr(ex: @ast::expr,
 
         // make sure that the thing we are pointing out stays valid
         // for the lifetime `scope_r` of the resulting ptr:
-        let scope_r =
-            match check ty::get(tcx.ty(ex)).struct {
-              ty::ty_rptr(r, _) => r
-            };
+        let scope_r = ty_region(tcx.ty(ex));
         self.guarantee_valid(base_cmt, mutbl, scope_r);
         visit::visit_expr(ex, self, vt);
       }
@@ -474,37 +472,60 @@ impl methods for gather_loan_ctxt {
             }
           }
 
-          ast::pat_ident(_, _, none) if self.pat_is_variant(pat) => {
-            // nullary variant
-            debug!{"nullary variant"};
-          }
-          ast::pat_ident(_, id, o_pat) => {
-            // XXX: Needs to take by-ref/by-val into account.
-
-            // x or x @ p --- `x` must remain valid for the scope of the alt
-            debug!{"defines identifier %s", pprust::path_to_str(id)};
-
-            // Note: there is a discussion of the function of
-            // cat_discr in the method preserve():
-            let cmt1 = self.bccx.cat_discr(cmt, alt_id);
-            let arm_scope = ty::re_scope(arm_id);
-
-            // Remember the mutability of the location that this
-            // binding refers to.  This will be used later when
-            // categorizing the binding.  This is a bit of a hack that
-            // would be better fixed by #2329; in that case we could
-            // allow the user to specify if they want an imm, const,
-            // or mut binding, or else just reflect the mutability
-            // through the type of the region pointer.
-            self.bccx.binding_map.insert(pat.id, cmt1.mutbl);
-
-            self.guarantee_valid(cmt1, m_const, arm_scope);
-
+          ast::pat_ident(bm, id, o_pat) if !self.pat_is_variant(pat) => {
+            match bm {
+              ast::bind_by_value => {
+                // copying does not borrow anything, so no check is required
+              }
+              ast::bind_by_ref(mutbl) => {
+                // ref x or ref x @ p --- creates a ptr which must
+                // remain valid for the scope of the alt
+
+                // find the region of the resulting pointer (note that
+                // the type of such a pattern will *always* be a
+                // region pointer)
+                let scope_r = ty_region(tcx.ty(pat));
+
+                // if the scope of the region ptr turns out to be
+                // specific to this arm, wrap the categorization with
+                // a cat_discr() node.  There is a detailed discussion
+                // of the function of this node in method preserve():
+                let arm_scope = ty::re_scope(arm_id);
+                if self.bccx.is_subregion_of(scope_r, arm_scope) {
+                    let cmt_discr = self.bccx.cat_discr(cmt, alt_id);
+                    self.guarantee_valid(cmt_discr, mutbl, scope_r);
+                } else {
+                    self.guarantee_valid(cmt, mutbl, scope_r);
+                }
+              }
+              ast::bind_by_implicit_ref => {
+                // Note: there is a discussion of the function of
+                // cat_discr in the method preserve():
+                let cmt1 = self.bccx.cat_discr(cmt, alt_id);
+                let arm_scope = ty::re_scope(arm_id);
+
+                // Remember the mutability of the location that this
+                // binding refers to.  This will be used later when
+                // categorizing the binding.  This is a bit of a hack that
+                // would be better fixed by #2329; in that case we could
+                // allow the user to specify if they want an imm, const,
+                // or mut binding, or else just reflect the mutability
+                // through the type of the region pointer.
+                self.bccx.binding_map.insert(pat.id, cmt1.mutbl);
+
+                self.guarantee_valid(cmt1, m_const, arm_scope);
+              }
+            }
             for o_pat.each |p| {
                 self.gather_pat(cmt, p, arm_id, alt_id);
             }
           }
 
+          ast::pat_ident(*) => {
+              // nullary variant: ignore.
+              assert self.pat_is_variant(pat);
+          }
+
           ast::pat_rec(field_pats, _) => {
             // {f1: p1, ..., fN: pN}
             for field_pats.each |fp| {

src/rustc/middle/borrowck/loan.rs

@@ -39,8 +39,7 @@ impl loan_methods for loan_ctxt {
     fn ok_with_loan_of(cmt: cmt,
                        scope_ub: ty::region,
                        mutbl: ast::mutability) -> bckres<()> {
-        let region_map = self.tcx().region_map;
-        if region::subregion(region_map, scope_ub, self.scope_region) {
+        if self.bccx.is_subregion_of(self.scope_region, scope_ub) {
             // Note: all cmt's that we deal with will have a non-none
             // lp, because the entry point into this routine,
             // `borrowck_ctxt::loan()`, rejects any cmt with a

src/rustc/middle/borrowck/preserve.rs

@@ -232,16 +232,6 @@ impl private_methods for &preserve_ctxt {
             // node appears to draw the line between what will be rooted
             // in the *arm* vs the *alt*.
 
-            // current scope must be the arm, which is always a child of alt:
-            assert {
-                match check self.scope_region {
-                  ty::re_scope(arm_id) => {
-                    self.tcx().region_map.get(arm_id) == alt_id
-                  }
-                  _ => {false}
-                }
-            };
-
             let alt_rooting_ctxt =
                 preserve_ctxt({scope_region: ty::re_scope(alt_id)
                                with **self});
@@ -289,8 +279,7 @@ impl private_methods for &preserve_ctxt {
     /// is a subscope of `scope_ub`; if so, success.
     fn compare_scope(cmt: cmt,
                      scope_ub: ty::region) -> bckres<preserve_condition> {
-        let region_map = self.tcx().region_map;
-        if region::subregion(region_map, scope_ub, self.scope_region) {
+        if self.bccx.is_subregion_of(self.scope_region, scope_ub) {
             ok(pc_ok)
         } else {
             err({cmt:cmt, code:err_out_of_scope(scope_ub,
@@ -326,12 +315,11 @@ impl private_methods for &preserve_ctxt {
           // we can only root values if the desired region is some concrete
           // scope within the fn body
           ty::re_scope(scope_id) => {
-            let region_map = self.tcx().region_map;
             #debug["Considering root map entry for %s: \
                     node %d:%u -> scope_id %?, root_ub %?",
                    self.bccx.cmt_to_repr(cmt), base.id,
                    derefs, scope_id, self.root_ub];
-            if region::subregion(region_map, root_region, self.scope_region) {
+            if self.bccx.is_subregion_of(self.scope_region, root_region) {
                 #debug["Elected to root"];
                 let rk = {id: base.id, derefs: derefs};
                 self.bccx.root_map.insert(rk, scope_id);

src/rustc/middle/region.rs

@@ -99,15 +99,17 @@ fn scope_contains(region_map: region_map, superscope: ast::node_id,
 /// Determines whether one region is a subregion of another.  This is
 /// intended to run *after inference* and sadly the logic is somewhat
 /// duplicated with the code in infer.rs.
-fn subregion(region_map: region_map,
-             super_region: ty::region,
-             sub_region: ty::region) -> bool {
-    super_region == sub_region ||
-        match (super_region, sub_region) {
-          (ty::re_static, _) => {true}
-
-          (ty::re_scope(super_scope), ty::re_scope(sub_scope)) |
-          (ty::re_free(super_scope, _), ty::re_scope(sub_scope)) => {
+fn is_subregion_of(region_map: region_map,
+                   sub_region: ty::region,
+                   super_region: ty::region) -> bool {
+    sub_region == super_region ||
+        match (sub_region, super_region) {
+          (_, ty::re_static) => {
+            true
+          }
+
+          (ty::re_scope(sub_scope), ty::re_scope(super_scope)) |
+          (ty::re_scope(sub_scope), ty::re_free(super_scope, _)) => {
             scope_contains(region_map, super_scope, sub_scope)
           }
 

src/rustc/middle/ty.rs

@@ -107,6 +107,7 @@ export tbox_has_flag;
 export ty_var_id;
 export ty_to_def_id;
 export ty_fn_args;
+export ty_region;
 export kind, kind_implicitly_copyable, kind_send_copy, kind_copyable;
 export kind_noncopyable, kind_const;
 export kind_can_be_copied, kind_can_be_sent, kind_can_be_implicitly_copied;
@@ -2297,8 +2298,15 @@ fn ty_fn_ret_style(fty: t) -> ast::ret_style {
 
 fn is_fn_ty(fty: t) -> bool {
     match get(fty).struct {
-      ty_fn(_) => return true,
-      _ => return false
+      ty_fn(_) => true,
+      _ => false
+    }
+}
+
+fn ty_region(ty: t) -> region {
+    match get(ty).struct {
+      ty_rptr(r, _) => r,
+      s => fail fmt!{"ty_region() invoked on non-rptr: %?", s}
     }
 }
 

src/test/compile-fail/borrowck-pat-by-value-binding.rs

@@ -0,0 +1,34 @@
+fn process<T>(_t: T) {}
+
+fn match_const_opt_by_mut_ref(v: &const option<int>) {
+    match *v {
+      some(ref mut i) => process(i), //~ ERROR illegal borrow
+      none => ()
+    }
+}
+
+fn match_const_opt_by_const_ref(v: &const option<int>) {
+    match *v {
+      some(ref const i) => process(i), //~ ERROR illegal borrow unless pure
+      //~^ NOTE impure due to
+      none => ()
+    }
+}
+
+fn match_const_opt_by_imm_ref(v: &const option<int>) {
+    match *v {
+      some(ref i) => process(i), //~ ERROR illegal borrow unless pure
+      //~^ NOTE impure due to
+      none => ()
+    }
+}
+
+fn match_const_opt_by_value(v: &const option<int>) {
+    match *v {
+      some(copy i) => process(i),
+      none => ()
+    }
+}
+
+fn main() {
+}

src/test/compile-fail/borrowck-ref-mut-of-imm.rs

@@ -0,0 +1,10 @@
+fn destructure(x: option<int>) -> int {
+    match x {
+      none => 0,
+      some(ref mut v) => *v //~ ERROR illegal borrow
+    }
+}
+
+fn main() {
+    assert destructure(some(22)) == 22;
+}