Distinguish tuple elements by index in mem_categorization. Fixes #5362.

Author: nikomatsakis

src/librustc/middle/borrowck/check_loans.rs

@@ -400,6 +400,7 @@ pub impl<'self> CheckLoanCtxt<'self> {
                         cmt = b;
                     }
 
+                    mc::cat_downcast(b) |
                     mc::cat_interior(b, _) => {
                         if cmt.mutbl == mc::McInherited {
                             cmt = b;

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

@@ -105,6 +105,7 @@ impl GuaranteeLifetimeContext {
                 }
             }
 
+            mc::cat_downcast(base) |
             mc::cat_deref(base, _, mc::uniq_ptr(*)) |
             mc::cat_interior(base, _) => {
                 self.check(base, discr_scope)
@@ -303,6 +304,7 @@ impl GuaranteeLifetimeContext {
             mc::cat_deref(*) => {
                 false
             }
+            r @ mc::cat_downcast(*) |
             r @ mc::cat_interior(*) |
             r @ mc::cat_stack_upvar(*) |
             r @ mc::cat_discr(*) => {
@@ -340,6 +342,7 @@ impl GuaranteeLifetimeContext {
             mc::cat_deref(_, _, mc::region_ptr(_, r)) => {
                 r
             }
+            mc::cat_downcast(cmt) |
             mc::cat_deref(cmt, _, mc::uniq_ptr(*)) |
             mc::cat_deref(cmt, _, mc::gc_ptr(*)) |
             mc::cat_interior(cmt, _) |

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

@@ -80,24 +80,17 @@ impl RestrictionsContext {
                                           set: restrictions}])
             }
 
-            mc::cat_interior(cmt_base, i @ mc::interior_variant(_)) => {
+            mc::cat_downcast(cmt_base) => {
                 // When we borrow the interior of an enum, we have to
                 // ensure the enum itself is not mutated, because that
                 // could cause the type of the memory to change.
-                let result = self.compute(cmt_base, restrictions | RESTR_MUTATE);
-                self.extend(result, cmt.mutbl, LpInterior(i), restrictions)
+                self.compute(cmt_base, restrictions | RESTR_MUTATE)
             }
 
-            mc::cat_interior(cmt_base, i @ mc::interior_tuple) |
-            mc::cat_interior(cmt_base, i @ mc::interior_anon_field) |
-            mc::cat_interior(cmt_base, i @ mc::interior_field(*)) |
-            mc::cat_interior(cmt_base, i @ mc::interior_index(*)) => {
-                // For all of these cases, overwriting the base would
-                // not change the type of the memory, so no additional
-                // restrictions are needed.
-                //
-                // FIXME(#5397) --- Mut fields are not treated soundly
-                //                  (hopefully they will just get phased out)
+            mc::cat_interior(cmt_base, i) => {
+                // Overwriting the base would not change the type of
+                // the memory, so no additional restrictions are
+                // needed.
                 let result = self.compute(cmt_base, restrictions);
                 self.extend(result, cmt.mutbl, LpInterior(i), restrictions)
             }

src/librustc/middle/borrowck/mod.rs

@@ -236,8 +236,8 @@ pub enum LoanPath {
 
 #[deriving(Eq)]
 pub enum LoanPathElem {
-    LpDeref,                      // `*LV` in doc.rs
-    LpInterior(mc::interior_kind) // `LV.f` in doc.rs
+    LpDeref,                     // `*LV` in doc.rs
+    LpInterior(mc::InteriorKind) // `LV.f` in doc.rs
 }
 
 pub impl LoanPath {
@@ -280,6 +280,7 @@ pub fn opt_loan_path(cmt: mc::cmt) -> Option<@LoanPath> {
                 |&lp| @LpExtend(lp, cmt.mutbl, LpInterior(ik)))
         }
 
+        mc::cat_downcast(cmt_base) |
         mc::cat_stack_upvar(cmt_base) |
         mc::cat_discr(cmt_base, _) => {
             opt_loan_path(cmt_base)
@@ -616,24 +617,25 @@ pub impl BorrowckCtxt {
                 }
             }
 
-            LpExtend(lp_base, _, LpInterior(mc::interior_field(fld))) => {
+            LpExtend(lp_base, _, LpInterior(mc::InteriorField(fname))) => {
                 self.append_loan_path_to_str_from_interior(lp_base, out);
-                str::push_char(out, '.');
-                str::push_str(out, *self.tcx.sess.intr().get(fld));
+                match fname {
+                    mc::NamedField(fname) => {
+                        str::push_char(out, '.');
+                        str::push_str(out, *self.tcx.sess.intr().get(fname));
+                    }
+                    mc::PositionalField(idx) => {
+                        str::push_char(out, '#'); // invent a notation here
+                        str::push_str(out, idx.to_str());
+                    }
+                }
             }
 
-            LpExtend(lp_base, _, LpInterior(mc::interior_index(*))) => {
+            LpExtend(lp_base, _, LpInterior(mc::InteriorElement(_))) => {
                 self.append_loan_path_to_str_from_interior(lp_base, out);
                 str::push_str(out, "[]");
             }
 
-            LpExtend(lp_base, _, LpInterior(mc::interior_tuple)) |
-            LpExtend(lp_base, _, LpInterior(mc::interior_anon_field)) |
-            LpExtend(lp_base, _, LpInterior(mc::interior_variant(_))) => {
-                self.append_loan_path_to_str_from_interior(lp_base, out);
-                str::push_str(out, ".(tuple)");
-            }
-
             LpExtend(lp_base, _, LpDeref) => {
                 str::push_char(out, '*');
                 self.append_loan_path_to_str(lp_base, out);

src/librustc/middle/mem_categorization.rs

@@ -66,9 +66,12 @@ pub enum categorization {
     cat_local(ast::node_id),           // local variable
     cat_arg(ast::node_id),             // formal argument
     cat_deref(cmt, uint, ptr_kind),    // deref of a ptr
-    cat_interior(cmt, interior_kind),  // something interior
+    cat_interior(cmt, InteriorKind),   // something interior: field, tuple, etc
+    cat_downcast(cmt),                 // selects a particular enum variant (*)
     cat_discr(cmt, ast::node_id),      // match discriminant (see preserve())
     cat_self(ast::node_id),            // explicit `self`
+
+    // (*) downcast is only required if the enum has more than one variant
 }
 
 #[deriving(Eq)]
@@ -89,14 +92,15 @@ pub enum ptr_kind {
 // We use the term "interior" to mean "something reachable from the
 // base without a pointer dereference", e.g. a field
 #[deriving(Eq)]
-pub enum interior_kind {
-    interior_tuple,                  // elt in a tuple
-    interior_anon_field,             // anonymous field (in e.g.
-                                     // struct Foo(int, int);
-    interior_variant(ast::def_id),   // internals to a variant of given enum
-    interior_field(ast::ident),      // name of field
-    interior_index(ty::t,            // type of vec/str/etc being deref'd
-                   ast::mutability)  // mutability of vec content
+pub enum InteriorKind {
+    InteriorField(FieldName),
+    InteriorElement(ty::t),    // ty::t is the type of the vec/str
+}
+
+#[deriving(Eq)]
+pub enum FieldName {
+    NamedField(ast::ident),
+    PositionalField(uint)
 }
 
 #[deriving(Eq)]
@@ -134,7 +138,10 @@ pub type cmt = @cmt_;
 
 // We pun on *T to mean both actual deref of a ptr as well
 // as accessing of components:
-pub enum deref_kind {deref_ptr(ptr_kind), deref_interior(interior_kind)}
+pub enum deref_kind {
+    deref_ptr(ptr_kind),
+    deref_interior(InteriorKind),
+}
 
 // Categorizes a derefable type.  Note that we include vectors and strings as
 // derefable (we model an index as the combination of a deref and then a
@@ -176,20 +183,14 @@ pub fn opt_deref_kind(t: ty::t) -> Option<deref_kind> {
             Some(deref_ptr(unsafe_ptr))
         }
 
-        ty::ty_enum(did, _) => {
-            Some(deref_interior(interior_variant(did)))
-        }
-
-        ty::ty_struct(_, _) => {
-            Some(deref_interior(interior_anon_field))
-        }
-
-        ty::ty_evec(mt, ty::vstore_fixed(_)) => {
-            Some(deref_interior(interior_index(t, mt.mutbl)))
+        ty::ty_enum(*) |
+        ty::ty_struct(*) => { // newtype
+            Some(deref_interior(InteriorField(PositionalField(0))))
         }
 
+        ty::ty_evec(_, ty::vstore_fixed(_)) |
         ty::ty_estr(ty::vstore_fixed(_)) => {
-            Some(deref_interior(interior_index(t, m_imm)))
+            Some(deref_interior(InteriorElement(t)))
         }
 
         _ => None
@@ -579,7 +580,7 @@ pub impl mem_categorization_ctxt {
         @cmt_ {
             id: node.id(),
             span: node.span(),
-            cat: cat_interior(base_cmt, interior_field(f_name)),
+            cat: cat_interior(base_cmt, InteriorField(NamedField(f_name))),
             mutbl: base_cmt.mutbl.inherit(),
             ty: f_ty
         }
@@ -737,15 +738,16 @@ pub impl mem_categorization_ctxt {
           }
         };
 
-        fn interior<N: ast_node>(elt: N, of_cmt: cmt,
-                                 vect: ty::t, mutbl: MutabilityCategory,
+        fn interior<N: ast_node>(elt: N,
+                                 of_cmt: cmt,
+                                 vec_ty: ty::t,
+                                 mutbl: MutabilityCategory,
                                  mt: ty::mt) -> cmt
         {
-            let interior = interior_index(vect, mt.mutbl);
             @cmt_ {
                 id:elt.id(),
                 span:elt.span(),
-                cat:cat_interior(of_cmt, interior),
+                cat:cat_interior(of_cmt, InteriorElement(vec_ty)),
                 mutbl:mutbl,
                 ty:mt.ty
             }
@@ -756,7 +758,7 @@ pub impl mem_categorization_ctxt {
                                     node: N,
                                     base_cmt: cmt,
                                     interior_ty: ty::t,
-                                    interior: interior_kind) -> cmt {
+                                    interior: InteriorKind) -> cmt {
         @cmt_ {
             id: node.id(),
             span: node.span(),
@@ -766,6 +768,19 @@ pub impl mem_categorization_ctxt {
         }
     }
 
+    fn cat_downcast<N:ast_node>(&self,
+                                node: N,
+                                base_cmt: cmt,
+                                downcast_ty: ty::t) -> cmt {
+        @cmt_ {
+            id: node.id(),
+            span: node.span(),
+            cat: cat_downcast(base_cmt),
+            mutbl: base_cmt.mutbl.inherit(),
+            ty: downcast_ty
+        }
+    }
+
     fn cat_pattern(&self,
                    cmt: cmt,
                    pat: @ast::pat,
@@ -835,21 +850,34 @@ pub impl mem_categorization_ctxt {
             match self.tcx.def_map.find(&pat.id) {
                 Some(&ast::def_variant(enum_did, _)) => {
                     // variant(x, y, z)
-                    for subpats.each |&subpat| {
+
+                    let downcast_cmt = {
+                        if ty::enum_is_univariant(tcx, enum_did) {
+                            cmt // univariant, no downcast needed
+                        } else {
+                            self.cat_downcast(pat, cmt, cmt.ty)
+                        }
+                    };
+
+                    for subpats.eachi |i, &subpat| {
                         let subpat_ty = self.pat_ty(subpat); // see (*)
+
                         let subcmt =
-                            self.cat_imm_interior(pat, cmt, subpat_ty,
-                                                  interior_variant(enum_did));
+                            self.cat_imm_interior(
+                                pat, downcast_cmt, subpat_ty,
+                                InteriorField(PositionalField(i)));
+
                         self.cat_pattern(subcmt, subpat, op);
                     }
                 }
                 Some(&ast::def_fn(*)) |
                 Some(&ast::def_struct(*)) => {
-                    for subpats.each |&subpat| {
+                    for subpats.eachi |i, &subpat| {
                         let subpat_ty = self.pat_ty(subpat); // see (*)
                         let cmt_field =
-                            self.cat_imm_interior(pat, cmt, subpat_ty,
-                                                  interior_anon_field);
+                            self.cat_imm_interior(
+                                pat, cmt, subpat_ty,
+                                InteriorField(PositionalField(i)));
                         self.cat_pattern(cmt_field, subpat, op);
                     }
                 }
@@ -885,10 +913,12 @@ pub impl mem_categorization_ctxt {
 
           ast::pat_tup(ref subpats) => {
             // (p1, ..., pN)
-            for subpats.each |&subpat| {
+            for subpats.eachi |i, &subpat| {
                 let subpat_ty = self.pat_ty(subpat); // see (*)
-                let subcmt = self.cat_imm_interior(pat, cmt, subpat_ty,
-                                                   interior_tuple);
+                let subcmt =
+                    self.cat_imm_interior(
+                        pat, cmt, subpat_ty,
+                        InteriorField(PositionalField(i)));
                 self.cat_pattern(subcmt, subpat, op);
             }
           }
@@ -931,22 +961,37 @@ pub impl mem_categorization_ctxt {
 
     fn cmt_to_str(&self, cmt: cmt) -> ~str {
         match cmt.cat {
-          cat_static_item => ~"static item",
-          cat_implicit_self => ~"self reference",
+          cat_static_item => {
+              ~"static item"
+          }
+          cat_implicit_self => {
+              ~"self reference"
+          }
           cat_copied_upvar(_) => {
               ~"captured outer variable in a heap closure"
           }
-          cat_rvalue => ~"non-lvalue",
-          cat_local(_) => ~"local variable",
-          cat_self(_) => ~"self value",
-          cat_arg(*) => ~"argument",
-          cat_deref(_, _, pk) => fmt!("dereference of %s pointer",
-                                      ptr_sigil(pk)),
-          cat_interior(_, interior_field(*)) => ~"field",
-          cat_interior(_, interior_tuple) => ~"tuple content",
-          cat_interior(_, interior_anon_field) => ~"anonymous field",
-          cat_interior(_, interior_variant(_)) => ~"enum content",
-          cat_interior(_, interior_index(t, _)) => {
+          cat_rvalue => {
+              ~"non-lvalue"
+          }
+          cat_local(_) => {
+              ~"local variable"
+          }
+          cat_self(_) => {
+              ~"self value"
+          }
+          cat_arg(*) => {
+              ~"argument"
+          }
+          cat_deref(_, _, pk) => {
+              fmt!("dereference of %s pointer", ptr_sigil(pk))
+          }
+          cat_interior(_, InteriorField(NamedField(_))) => {
+              ~"field"
+          }
+          cat_interior(_, InteriorField(PositionalField(_))) => {
+              ~"anonymous field"
+          }
+          cat_interior(_, InteriorElement(t)) => {
             match ty::get(t).sty {
               ty::ty_evec(*) => ~"vec content",
               ty::ty_estr(*) => ~"str content",
@@ -959,6 +1004,9 @@ pub impl mem_categorization_ctxt {
           cat_discr(cmt, _) => {
             self.cmt_to_str(cmt)
           }
+          cat_downcast(cmt) => {
+            self.cmt_to_str(cmt)
+          }
         }
     }
 
@@ -1027,6 +1075,7 @@ pub impl cmt_ {
             cat_deref(_, _, region_ptr(*)) => {
                 self
             }
+            cat_downcast(b) |
             cat_stack_upvar(b) |
             cat_discr(b, _) |
             cat_interior(b, _) |
@@ -1075,6 +1124,7 @@ pub impl cmt_ {
                 Some(AliasableBorrowed(m))
             }
 
+            cat_downcast(b) |
             cat_stack_upvar(b) |
             cat_deref(b, _, uniq_ptr(*)) |
             cat_interior(b, _) |
@@ -1114,6 +1164,9 @@ impl Repr for categorization {
                      cmt.cat.repr(tcx),
                      interior.repr(tcx))
             }
+            cat_downcast(cmt) => {
+                fmt!("%s->(enum)", cmt.cat.repr(tcx))
+            }
             cat_stack_upvar(cmt) |
             cat_discr(cmt, _) => cmt.cat.repr(tcx)
         }
@@ -1129,14 +1182,12 @@ pub fn ptr_sigil(ptr: ptr_kind) -> ~str {
     }
 }
 
-impl Repr for interior_kind {
+impl Repr for InteriorKind {
     fn repr(&self, tcx: ty::ctxt) -> ~str {
         match *self {
-            interior_field(fld) => copy *tcx.sess.str_of(fld),
-            interior_index(*) => ~"[]",
-            interior_tuple => ~"()",
-            interior_anon_field => ~"<anonymous field>",
-            interior_variant(_) => ~"<enum>"
+            InteriorField(NamedField(fld)) => copy *tcx.sess.str_of(fld),
+            InteriorField(PositionalField(i)) => fmt!("#%?", i),
+            InteriorElement(_) => ~"[]",
         }
     }
 }

src/test/compile-fail/borrowck-anon-fields-struct.rs

@@ -0,0 +1,37 @@
+// Tests that we are able to distinguish when loans borrow different
+// anonymous fields of an tuple vs the same anonymous field.
+
+struct Y(uint, uint);
+
+fn distinct_variant() {
+    let mut y = Y(1, 2);
+
+    let a = match y {
+        Y(ref mut a, _) => a
+    };
+
+    let b = match y {
+        Y(_, ref mut b) => b
+    };
+
+    *a += 1;
+    *b += 1;
+}
+
+fn same_variant() {
+    let mut y = Y(1, 2);
+
+    let a = match y {
+        Y(ref mut a, _) => a
+    };
+
+    let b = match y {
+        Y(ref mut b, _) => b //~ ERROR cannot borrow
+    };
+
+    *a += 1;
+    *b += 1;
+}
+
+fn main() {
+}

src/test/compile-fail/borrowck-anon-fields-tuple.rs

@@ -0,0 +1,35 @@
+// Tests that we are able to distinguish when loans borrow different
+// anonymous fields of a tuple vs the same anonymous field.
+
+fn distinct_variant() {
+    let mut y = (1, 2);
+
+    let a = match y {
+        (ref mut a, _) => a
+    };
+
+    let b = match y {
+        (_, ref mut b) => b
+    };
+
+    *a += 1;
+    *b += 1;
+}
+
+fn same_variant() {
+    let mut y = (1, 2);
+
+    let a = match y {
+        (ref mut a, _) => a
+    };
+
+    let b = match y {
+        (ref mut b, _) => b //~ ERROR cannot borrow
+    };
+
+    *a += 1;
+    *b += 1;
+}
+
+fn main() {
+}

src/test/compile-fail/borrowck-anon-fields-variant.rs

@@ -0,0 +1,43 @@
+// Tests that we are able to distinguish when loans borrow different
+// anonymous fields of an enum variant vs the same anonymous field.
+
+enum Foo {
+    X, Y(uint, uint)
+}
+
+fn distinct_variant() {
+    let mut y = Y(1, 2);
+
+    let a = match y {
+      Y(ref mut a, _) => a,
+      X => fail!()
+    };
+
+    let b = match y {
+      Y(_, ref mut b) => b,
+      X => fail!()
+    };
+
+    *a += 1;
+    *b += 1;
+}
+
+fn same_variant() {
+    let mut y = Y(1, 2);
+
+    let a = match y {
+      Y(ref mut a, _) => a,
+      X => fail!()
+    };
+
+    let b = match y {
+      Y(ref mut b, _) => b, //~ ERROR cannot borrow
+      X => fail!()
+    };
+
+    *a += 1;
+    *b += 1;
+}
+
+fn main() {
+}