Treat enums with one variant specially in borrowck: #2573

Author: nikomatsakis

src/rustc/middle/borrowck.rs

@@ -238,11 +238,13 @@ enum ptr_kind {uniq_ptr, gc_ptr, region_ptr, unsafe_ptr}
 // I am coining the term "components" to mean "pieces of a data
 // structure accessible without a dereference":
 enum comp_kind {
-    comp_tuple, comp_res, comp_variant,
-    comp_field(str, // name of field
+    comp_tuple,                  // elt in a tuple
+    comp_res,                    // data for a resource
+    comp_variant(ast::def_id),   // internals to a variant of given enum
+    comp_field(str,              // name of field
                ast::mutability), // declared mutability of field
-    comp_index(ty::t, // type of vec/str/etc being deref'd
-               ast::mutability) // mutability of vec content
+    comp_index(ty::t,            // type of vec/str/etc being deref'd
+               ast::mutability)  // mutability of vec content
 }
 
 // We pun on *T to mean both actual deref of a ptr as well
@@ -411,7 +413,7 @@ impl to_str_methods for borrowck_ctxt {
           comp_index(*) { "[]" }
           comp_tuple { "()" }
           comp_res { "<res>" }
-          comp_variant { "<enum>" }
+          comp_variant(_) { "<enum>" }
         }
     }
 
@@ -468,7 +470,7 @@ impl to_str_methods for borrowck_ctxt {
           cat_comp(_, comp_field(*)) { mut_str + " field" }
           cat_comp(_, comp_tuple) { "tuple content" }
           cat_comp(_, comp_res) { "resource content" }
-          cat_comp(_, comp_variant) { "enum content" }
+          cat_comp(_, comp_variant(_)) { "enum content" }
           cat_comp(_, comp_index(t, _)) {
             alt ty::get(t).struct {
               ty::ty_vec(*) | ty::ty_evec(*) {
@@ -514,7 +516,7 @@ impl to_str_methods for borrowck_ctxt {
 // mutable structure.
 fn inherent_mutability(ck: comp_kind) -> mutability {
     alt ck {
-      comp_tuple | comp_res | comp_variant {m_imm}
+      comp_tuple | comp_res | comp_variant(_) {m_imm}
       comp_field(_, m) | comp_index(_, m) {m}
     }
 }

src/rustc/middle/borrowck/categorization.rs

@@ -67,8 +67,8 @@ fn opt_deref_kind(t: ty::t) -> option<deref_kind> {
         some(deref_ptr(unsafe_ptr))
       }
 
-      ty::ty_enum(*) {
-        some(deref_comp(comp_variant))
+      ty::ty_enum(did, _) {
+        some(deref_comp(comp_variant(did)))
       }
 
       ty::ty_res(*) {
@@ -275,10 +275,12 @@ impl public_methods for borrowck_ctxt {
         }
     }
 
-    fn cat_variant<N: ast_node>(arg: N, cmt: cmt) -> cmt {
+    fn cat_variant<N: ast_node>(arg: N,
+                                enum_did: ast::def_id,
+                                cmt: cmt) -> cmt {
         @{id: arg.id(), span: arg.span(),
-          cat: cat_comp(cmt, comp_variant),
-          lp: cmt.lp.map { |l| @lp_comp(l, comp_variant) },
+          cat: cat_comp(cmt, comp_variant(enum_did)),
+          lp: cmt.lp.map { |l| @lp_comp(l, comp_variant(enum_did)) },
           mutbl: cmt.mutbl, // imm iff in an immutable context
           ty: self.tcx.ty(arg)}
     }

src/rustc/middle/borrowck/gather_loans.rs

@@ -338,8 +338,16 @@ impl methods for gather_loan_ctxt {
           }
           ast::pat_enum(_, some(subpats)) {
             // variant(x, y, z)
+            let enum_did = alt self.bccx.tcx.def_map
+.find(pat.id) {
+              some(ast::def_variant(enum_did, _)) {enum_did}
+              e {tcx.sess.span_bug(pat.span,
+                                   #fmt["resolved to %?, \
+                                         not variant", e])}
+            };
+
             for subpats.each { |subpat|
-                let subcmt = self.bccx.cat_variant(subpat, cmt);
+                let subcmt = self.bccx.cat_variant(subpat, enum_did, cmt);
                 self.gather_pat(subcmt, subpat, arm_id, alt_id);
             }
           }

src/rustc/middle/borrowck/loan.rs

@@ -65,24 +65,23 @@ impl loan_methods for loan_ctxt {
             // 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.
-            let base_mutbl = alt req_mutbl {
-              m_imm { m_imm }
-              m_const | m_mutbl { m_const }
-            };
-
-            self.loan(cmt_base, base_mutbl);
-            self.ok_with_loan_of(cmt, req_mutbl)
+            self.loan_stable_comp(cmt, cmt_base, req_mutbl)
           }
-          cat_comp(cmt1, comp_variant) |
-          cat_deref(cmt1, _, uniq_ptr) {
-            // Variant components: the base must be immutable, because
-            // if it is overwritten, the types of the embedded data
-            // could change.
-            //
-            // Unique pointers: the base must be immutable, because if
-            // it is overwritten, the unique content will be freed.
-            self.loan(cmt1, m_imm);
-            self.ok_with_loan_of(cmt, req_mutbl)
+          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)
+            } else {
+                self.loan_unstable_deref(cmt, cmt_base, req_mutbl)
+            }
+          }
+          cat_deref(cmt_base, _, uniq_ptr) {
+            // For unique pointers, the memory being pointed out is
+            // unstable because if the unique pointer is overwritten
+            // then the memory is freed.
+            self.loan_unstable_deref(cmt, cmt_base, req_mutbl)
           }
           cat_deref(cmt1, _, unsafe_ptr) |
           cat_deref(cmt1, _, gc_ptr) |
@@ -94,4 +93,32 @@ impl loan_methods for loan_ctxt {
           }
         }
     }
+
+    // A "stable component" is one where assigning the base of the
+    // component cannot cause the component itself to change types.
+    // Example: record fields.
+    fn loan_stable_comp(cmt: cmt,
+                        cmt_base: cmt,
+                        req_mutbl: ast::mutability) {
+        let base_mutbl = alt req_mutbl {
+          m_imm { m_imm }
+          m_const | m_mutbl { m_const }
+        };
+
+        self.loan(cmt_base, base_mutbl);
+        self.ok_with_loan_of(cmt, req_mutbl)
+    }
+
+    // An "unstable deref" means a deref of a ptr/comp where, if the
+    // base of the deref is assigned to, pointers into the result of the
+    // deref would be invalidated. Examples: interior of variants, uniques.
+    fn loan_unstable_deref(cmt: cmt,
+                           cmt_base: cmt,
+                           req_mutbl: ast::mutability) {
+        // Variant components: the base must be immutable, because
+        // if it is overwritten, the types of the embedded data
+        // could change.
+        self.loan(cmt_base, m_imm);
+        self.ok_with_loan_of(cmt, req_mutbl)
+    }
 }

src/rustc/middle/borrowck/preserve.rs

@@ -43,10 +43,19 @@ impl public_methods for borrowck_ctxt {
             // type never changes.
             self.preserve(cmt_base, opt_scope_id)
           }
-          cat_comp(cmt_base, comp_variant) {
-            self.require_imm(cmt, cmt_base, opt_scope_id, err_mut_variant)
+          cat_comp(cmt_base, comp_variant(enum_did)) {
+            if ty::enum_is_univariant(self.tcx, enum_did) {
+                self.preserve(cmt_base, opt_scope_id)
+            } else {
+                // If there are multiple variants: overwriting the
+                // base could cause the type of this memory to change,
+                // so require imm.
+                self.require_imm(cmt, cmt_base, opt_scope_id, err_mut_variant)
+            }
           }
           cat_deref(cmt_base, _, uniq_ptr) {
+            // Overwriting the base could cause this memory to be
+            // freed, so require imm.
             self.require_imm(cmt, cmt_base, opt_scope_id, err_mut_uniq)
           }
           cat_deref(_, _, region_ptr) {

src/rustc/middle/ty.rs

@@ -68,7 +68,7 @@ export sty;
 export subst, subst_tps, substs_is_noop, substs_to_str, substs;
 export t;
 export new_ty_hash;
-export enum_variants, substd_enum_variants;
+export enum_variants, substd_enum_variants, enum_is_univariant;
 export iface_methods, store_iface_methods, impl_iface;
 export enum_variant_with_id;
 export ty_dtor;
@@ -2663,6 +2663,10 @@ fn item_path(cx: ctxt, id: ast::def_id) -> ast_map::path {
     }
 }
 
+fn enum_is_univariant(cx: ctxt, id: ast::def_id) -> bool {
+    vec::len(*enum_variants(cx, id)) == 1u
+}
+
 fn enum_variants(cx: ctxt, id: ast::def_id) -> @[variant_info] {
     alt cx.enum_var_cache.find(id) {
       some(variants) { ret variants; }

src/test/run-pass/borrowck-newtype-issue-2573.rs

@@ -0,0 +1,16 @@
+enum foo = {mut bar: baz};
+
+enum baz = @{mut baz: int};
+
+impl quuux for foo {
+    fn frob() {
+        really_impure(self.bar);
+    }
+}
+
+// Override default mode so that we are passing by value
+fn really_impure(++bar: baz) {
+    bar.baz = 3;
+}
+
+fn main() {}

src/test/run-pass/borrowck-univariant-enum.rs

@@ -0,0 +1,19 @@
+enum newtype {
+    newtype(int)
+}
+
+fn main() {
+
+    // Test that borrowck treats enums with a single variant
+    // specially.
+
+    let x = @mut 5;
+    let y = @mut newtype(3);
+    let z = alt *y {
+      newtype(b) {
+        *x += 1;
+        *x * b
+      }
+    };
+    assert z == 18;
+}