Correct propagation of mutability from components to base in borrowck

Fixes #3828.

src/rustc/middle/borrowck/loan.rs

@@ -106,23 +106,26 @@ impl LoanContext {
           cat_discr(base, _) => {
             self.loan(base, req_mutbl)
           }
-          cat_comp(cmt_base, comp_field(*)) |
+          cat_comp(cmt_base, comp_field(_, m)) |
-          cat_comp(cmt_base, comp_index(*)) |
+          cat_comp(cmt_base, comp_index(_, m)) => {
-          cat_comp(cmt_base, comp_tuple) => {
             // For most components, the type of the embedded data is
             // stable.  Therefore, the base structure need only be
             // const---unless the component must be immutable.  In
             // that case, it must also be embedded in an immutable
             // location, or else the whole structure could be
             // overwritten and the component along with it.
-            self.loan_stable_comp(cmt, cmt_base, req_mutbl)
+            self.loan_stable_comp(cmt, cmt_base, req_mutbl, m)
+          }
+          cat_comp(cmt_base, comp_tuple) => {
+            // As above.
+            self.loan_stable_comp(cmt, cmt_base, req_mutbl, m_imm)
           }
           cat_comp(cmt_base, comp_variant(enum_did)) => {
             // For enums, the memory is unstable if there are multiple
             // variants, because if the enum value is overwritten then
             // the memory changes type.
             if ty::enum_is_univariant(self.bccx.tcx, enum_did) {
-                self.loan_stable_comp(cmt, cmt_base, req_mutbl)
+                self.loan_stable_comp(cmt, cmt_base, req_mutbl, m_imm)
             } else {
                 self.loan_unstable_deref(cmt, cmt_base, req_mutbl)
             }
@@ -150,10 +153,59 @@ impl LoanContext {
     fn loan_stable_comp(&self,
                         cmt: cmt,
                         cmt_base: cmt,
-                        req_mutbl: ast::mutability) -> bckres<()> {
+                        req_mutbl: ast::mutability,
-        let base_mutbl = match req_mutbl {
+                        comp_mutbl: ast::mutability) -> bckres<()> {
-          m_imm => m_imm,
+        // Determine the mutability that the base component must have,
-          m_const | m_mutbl => m_const
+        // given the required mutability of the pointer (`req_mutbl`)
+        // and the declared mutability of the component (`comp_mutbl`).
+        // This is surprisingly subtle.
+        //
+        // Note that the *declared* mutability of the component is not
+        // necessarily the same as cmt.mutbl, since a component
+        // declared as immutable but embedded in a mutable context
+        // becomes mutable.  It's best to think of comp_mutbl as being
+        // either MUTABLE or DEFAULT, not MUTABLE or IMMUTABLE.  We
+        // should really patch up the AST to reflect this distinction.
+        //
+        // Let's consider the cases below:
+        //
+        // 1. mut required, mut declared: In this case, the base
+        //    component must merely be const.  The reason is that it
+        //    does not matter if the base component is borrowed as
+        //    mutable or immutable, as the mutability of the base
+        //    component is overridden in the field declaration itself
+        //    (see `compile-fail/borrowck-mut-field-imm-base.rs`)
+        //
+        // 2. mut required, imm declared: This would only be legal if
+        //    the component is embeded in a mutable context.  However,
+        //    we detect mismatches between the mutability of the value
+        //    as a whole and the required mutability in `issue_loan()`
+        //    above.  In any case, presuming that the component IS
+        //    embedded in a mutable context, both the component and
+        //    the base must be loaned as MUTABLE.  This is to ensure
+        //    that there is no loan of the base as IMMUTABLE, which
+        //    would imply that the component must be IMMUTABLE too
+        //    (see `compile-fail/borrowck-imm-field-imm-base.rs`).
+        //
+        // 3. mut required, const declared: this shouldn't really be
+        //    possible, since I don't think you can declare a const
+        //    field, but I guess if we DID permit such a declaration
+        //    it would be equivalent to the case above?
+        //
+        // 4. imm required, * declared: In this case, the base must be
+        //    immutable.  This is true regardless of what was declared
+        //    for this subcomponent, this if the base is mutable, the
+        //    subcomponent must be mutable.
+        //    (see `compile-fail/borrowck-imm-field-mut-base.rs`).
+        //
+        // 5. const required, * declared: In this case, the base need
+        //    only be const, since we don't ultimately care whether
+        //    the subcomponent is mutable or not.
+        let base_mutbl = match (req_mutbl, comp_mutbl) {
+            (m_mutbl, m_mutbl) => m_const, // (1)
+            (m_mutbl, _) => m_mutbl,       // (2, 3)
+            (m_imm, _) => m_imm,           // (4)
+            (m_const, _) => m_const        // (5)
         };
 
         do self.loan(cmt_base, base_mutbl).chain |_ok| {

src/test/compile-fail/borrowck-imm-field-imm-base.rs

@@ -0,0 +1,17 @@
+struct Foo {
+    x: uint
+}
+
+struct Bar {
+    foo: Foo
+}
+
+fn main() {
+    let mut b = Bar { foo: Foo { x: 3 } };
+    let p = &b; //~ NOTE prior loan as immutable granted here
+    let q = &mut b.foo.x; //~ ERROR loan of mutable local variable as mutable conflicts with prior loan
+    let r = &p.foo.x;
+    io::println(fmt!("*r = %u", *r));
+    *q += 1;
+    io::println(fmt!("*r = %u", *r));
+}

src/test/compile-fail/borrowck-imm-field-mut-base.rs

@@ -0,0 +1,19 @@
+struct Foo {
+    mut x: uint
+}
+
+struct Bar {
+    foo: Foo
+}
+
+fn main() {
+    let mut b = Bar { foo: Foo { x: 3 } };
+    let p = &b.foo.x;
+    let q = &mut b.foo; //~ ERROR loan of mutable field as mutable conflicts with prior loan
+    //~^ ERROR loan of mutable local variable as mutable conflicts with prior loan
+    let r = &mut b; //~ ERROR loan of mutable local variable as mutable conflicts with prior loan
+    io::println(fmt!("*p = %u", *p));
+    q.x += 1;
+    r.foo.x += 1;
+    io::println(fmt!("*p = %u", *p));
+}

src/test/compile-fail/borrowck-mut-field-imm-base.rs

@@ -0,0 +1,20 @@
+struct Foo {
+    mut x: uint
+}
+
+struct Bar {
+    foo: Foo
+}
+
+fn main() {
+    let mut b = Bar { foo: Foo { x: 3 } };
+    let p = &b;
+    let q = &mut b.foo.x;
+    let r = &p.foo.x; //~ ERROR illegal borrow unless pure
+    let s = &b.foo.x; //~ ERROR loan of mutable field as immutable conflicts with prior loan
+    io::println(fmt!("*r = %u", *r));
+    io::println(fmt!("*r = %u", *s));
+    *q += 1;
+    io::println(fmt!("*r = %u", *r));
+    io::println(fmt!("*r = %u", *s));
+}