From 42c05fe642efa726dc6cde624b40b638741724ee Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 21 Oct 2012 23:02:02 -0700 Subject: [PATCH] Correct propagation of mutability from components to base in borrowck Fixes #3828. --- src/rustc/middle/borrowck/loan.rs | 70 ++++++++++++++++--- .../borrowck-imm-field-imm-base.rs | 17 +++++ .../borrowck-imm-field-mut-base.rs | 19 +++++ .../borrowck-mut-field-imm-base.rs | 20 ++++++ 4 files changed, 117 insertions(+), 9 deletions(-) create mode 100644 src/test/compile-fail/borrowck-imm-field-imm-base.rs create mode 100644 src/test/compile-fail/borrowck-imm-field-mut-base.rs create mode 100644 src/test/compile-fail/borrowck-mut-field-imm-base.rs diff --git a/src/rustc/middle/borrowck/loan.rs b/src/rustc/middle/borrowck/loan.rs index 7f4f857dae8..71414e6e724 100644 --- a/src/rustc/middle/borrowck/loan.rs +++ b/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_index(*)) | - cat_comp(cmt_base, comp_tuple) => { + cat_comp(cmt_base, comp_field(_, m)) | + cat_comp(cmt_base, comp_index(_, m)) => { // 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<()> { - let base_mutbl = match req_mutbl { - m_imm => m_imm, - m_const | m_mutbl => m_const + req_mutbl: ast::mutability, + comp_mutbl: ast::mutability) -> bckres<()> { + // Determine the mutability that the base component must have, + // 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| { diff --git a/src/test/compile-fail/borrowck-imm-field-imm-base.rs b/src/test/compile-fail/borrowck-imm-field-imm-base.rs new file mode 100644 index 00000000000..69ff3b10378 --- /dev/null +++ b/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)); +} \ No newline at end of file diff --git a/src/test/compile-fail/borrowck-imm-field-mut-base.rs b/src/test/compile-fail/borrowck-imm-field-mut-base.rs new file mode 100644 index 00000000000..6b9d93462d9 --- /dev/null +++ b/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)); +} \ No newline at end of file diff --git a/src/test/compile-fail/borrowck-mut-field-imm-base.rs b/src/test/compile-fail/borrowck-mut-field-imm-base.rs new file mode 100644 index 00000000000..f13637ab86a --- /dev/null +++ b/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)); +} \ No newline at end of file