Enforce mutability declarations in classes; correct shapes for classes

1. Enforce mutability declarations on class fields. Don't allow any
mutation of class fields not declared as mutable (except inside the
constructor).

2. Handle classes correctly in shape (treat classes like records).

Author: catamorphism

src/rustc/metadata/common.rs

@@ -78,6 +78,7 @@ const tag_path_len: uint = 0x41u;
 const tag_path_elt_mod: uint = 0x42u;
 const tag_path_elt_name: uint = 0x43u;
 const tag_item_field: uint = 0x44u;
+const tag_class_mut: uint = 0x45u;
 
 // used to encode crate_ctxt side tables
 enum astencode_tag { // Reserves 0x50 -- 0x6f

src/rustc/metadata/decoder.rs

@@ -118,6 +118,18 @@ fn class_member_id(d: ebml::doc, cdata: cmd) -> ast::def_id {
     ret translate_def_id(cdata, parse_def_id(ebml::doc_data(tagdoc)));
 }
 
+fn field_mutability(d: ebml::doc) -> ast::class_mutability {
+    // Use maybe_get_doc in case it's a method
+    option::maybe(ebml::maybe_get_doc(d, tag_class_mut),
+                  ast::class_immutable,
+                  {|d|
+                  alt ebml::doc_as_u8(d) as char {
+                    'm' { ast::class_mutable }
+                    _   { ast::class_immutable }
+                  }
+                  })
+}
+
 fn variant_disr_val(d: ebml::doc) -> option<int> {
     option::chain(ebml::maybe_get_doc(d, tag_disr_val)) {|val_doc|
         int::parse_buf(ebml::doc_data(val_doc), 10u)
@@ -435,9 +447,9 @@ fn get_class_members(cdata: cmd, id: ast::node_id,
        if p(f) {
           let name = item_name(an_item);
           let did = class_member_id(an_item, cdata);
+          let mt = field_mutability(an_item);
           result += [{ident: name, id: did, privacy:
-                  // This won't work for methods, argh
-                  family_to_privacy(f)}];
+                  family_to_privacy(f), mutability: mt}];
        }
     }
     result

src/rustc/metadata/encoder.rs

@@ -47,6 +47,13 @@ fn encode_named_def_id(ebml_w: ebml::writer, name: str, id: def_id) {
     }
 }
 
+fn encode_mutability(ebml_w: ebml::writer, mt: class_mutability) {
+    ebml_w.wr_tag(tag_class_mut) {||
+        ebml_w.writer.write([alt mt { class_immutable { 'i' }
+                class_mutable { 'm' } } as u8]);
+        }
+}
+
 type entry<T> = {val: T, pos: uint};
 
 fn encode_enum_variant_paths(ebml_w: ebml::writer, variants: [variant],
@@ -370,15 +377,15 @@ fn encode_info_for_class(ecx: @encode_ctxt, ebml_w: ebml::writer,
      /* We encode both private and public fields -- need to include
         private fields to get the offsets right */
       alt ci.node.decl {
-        instance_var(nm, _, _, id) {
+        instance_var(nm, _, mt, id) {
           *index += [{val: id, pos: ebml_w.writer.tell()}];
           ebml_w.start_tag(tag_items_data_item);
           #debug("encode_info_for_class: doing %s %d", nm, id);
           encode_privacy(ebml_w, ci.node.privacy);
           encode_name(ebml_w, nm);
           encode_path(ebml_w, path, ast_map::path_name(nm));
           encode_type(ecx, ebml_w, node_id_to_type(tcx, id));
-          /* TODO: mutability */
+          encode_mutability(ebml_w, mt);
           encode_def_id(ebml_w, local_def(id));
           ebml_w.end_tag();
         }

src/rustc/middle/mutbl.rs

@@ -57,6 +57,16 @@ fn expr_root(tcx: ty::ctxt, ex: @expr, autoderef: bool) ->
                     }
                 }
               }
+              ty::ty_class(did, _) {
+                  util::common::log_expr(*ex);
+                  for fld: ty::field_ty in ty::lookup_class_fields(tcx, did) {
+                    #debug("%s %?", fld.ident, fld.mutability);
+                    if str::eq(ident, fld.ident) {
+                        is_mutbl = fld.mutability == class_mutable;
+                    }
+                    break;
+                  }
+              }
               _ {}
             }
             ds += [@{mutbl: is_mutbl, kind: field, outer_t: auto_unbox.t}];
@@ -114,14 +124,17 @@ fn expr_root(tcx: ty::ctxt, ex: @expr, autoderef: bool) ->
 // Actual mutbl-checking pass
 
 type mutbl_map = std::map::hashmap<node_id, ()>;
-type ctx = {tcx: ty::ctxt, mutbl_map: mutbl_map};
+// Keep track of whether we're inside a ctor, so as to
+// allow mutating immutable fields in the same class
+type ctx = {tcx: ty::ctxt, mutbl_map: mutbl_map, in_ctor: bool};
 
 fn check_crate(tcx: ty::ctxt, crate: @crate) -> mutbl_map {
-    let cx = @{tcx: tcx, mutbl_map: std::map::int_hash()};
-    let v = @{visit_expr: bind visit_expr(cx, _, _, _),
-              visit_decl: bind visit_decl(cx, _, _, _)
+    let cx = @{tcx: tcx, mutbl_map: std::map::int_hash(), in_ctor: false};
+    let v = @{visit_expr: visit_expr,
+              visit_decl: visit_decl,
+              visit_item: visit_item
               with *visit::default_visitor()};
-    visit::visit_crate(*crate, (), visit::mk_vt(v));
+    visit::visit_crate(*crate, cx, visit::mk_vt(v));
     ret cx.mutbl_map;
 }
 
@@ -135,8 +148,8 @@ fn mk_err(cx: @ctx, span: syntax::codemap::span, msg: msg, name: str) {
     });
 }
 
-fn visit_decl(cx: @ctx, d: @decl, &&e: (), v: visit::vt<()>) {
-    visit::visit_decl(d, e, v);
+fn visit_decl(d: @decl, &&cx: @ctx, v: visit::vt<@ctx>) {
+    visit::visit_decl(d, cx, v);
     alt d.node {
       decl_local(locs) {
         for loc in locs {
@@ -152,7 +165,7 @@ fn visit_decl(cx: @ctx, d: @decl, &&e: (), v: visit::vt<()>) {
     }
 }
 
-fn visit_expr(cx: @ctx, ex: @expr, &&e: (), v: visit::vt<()>) {
+fn visit_expr(ex: @expr, &&cx: @ctx, v: visit::vt<@ctx>) {
     alt ex.node {
       expr_call(f, args, _) { check_call(cx, f, args); }
       expr_bind(f, args) { check_bind(cx, f, args); }
@@ -179,7 +192,22 @@ fn visit_expr(cx: @ctx, ex: @expr, &&e: (), v: visit::vt<()>) {
       }
       _ { }
     }
-    visit::visit_expr(ex, e, v);
+    visit::visit_expr(ex, cx, v);
+}
+
+fn visit_item(item: @item, &&cx: @ctx, v: visit::vt<@ctx>) {
+    alt item.node {
+            item_class(tps, items, ctor) {
+                v.visit_ty_params(tps, cx, v);
+                vec::map::<@class_item, ()>(items,
+                      {|i| v.visit_class_item(i.span,
+                            i.node.privacy, i.node.decl, cx, v); });
+                v.visit_fn(visit::fk_ctor(item.ident, tps), ctor.node.dec,
+                           ctor.node.body, ctor.span, ctor.node.id,
+                           @{in_ctor: true with *cx}, v);
+            }
+            _ { visit::visit_item(item, cx, v); }
+    }
 }
 
 fn check_lval(cx: @ctx, dest: @expr, msg: msg) {
@@ -277,7 +305,7 @@ fn check_bind(cx: @ctx, f: @expr, args: [option<@expr>]) {
 fn is_illegal_to_modify_def(cx: @ctx, def: def, msg: msg) -> option<str> {
     alt def {
       def_fn(_, _) | def_mod(_) | def_native_mod(_) | def_const(_) |
-      def_use(_) {
+      def_use(_) | def_class_method(_,_) {
         some("static item")
       }
       def_arg(_, m) {
@@ -310,6 +338,18 @@ fn is_illegal_to_modify_def(cx: @ctx, def: def, msg: msg) -> option<str> {
       }
 
       def_binding(_) { some("binding") }
+      def_class_field(parent,fld) {
+          if !cx.in_ctor {
+             /* Enforce mutability *unless* we're inside a ctor */
+             alt ty::lookup_class_field(cx.tcx, parent, fld).mutability {
+               class_mutable { none }
+               class_immutable { some("immutable class field") }
+             }
+          }
+          else {
+              none
+          }
+      }
       _ { none }
     }
 }

src/rustc/middle/trans/base.rs

@@ -2271,11 +2271,7 @@ fn trans_rec_field_inner(bcx: block, val: ValueRef, ty: ty::t,
             _ { bcx.tcx().sess.span_bug(sp, "trans_rec_field:\
                  base expr has non-record type"); }
         };
-    let ix = alt ty::field_idx(field, fields) {
-      none { bcx.tcx().sess.span_bug(sp, #fmt("trans_rec_field:\
-               base expr doesn't appear to have a field named %s", field));}
-      some(i) { i }
-    };
+    let ix = field_idx_strict(bcx.tcx(), sp, field, fields);
     let val = GEPi(bcx, val, [0, ix as int]);
     ret {bcx: bcx, val: val, kind: owned};
 }
@@ -3666,7 +3662,7 @@ fn raw_block(fcx: fn_ctxt, llbb: BasicBlockRef) -> block {
 // trans_block_cleanups: Go through all the cleanups attached to this
 // block and execute them.
 //
-// When translating a block that introdces new variables during its scope, we
+// When translating a block that introduces new variables during its scope, we
 // need to make sure those variables go out of scope when the block ends.  We
 // do that by running a 'cleanup' function for each variable.
 // trans_block_cleanups runs all the cleanup functions for the block.
@@ -4344,10 +4340,24 @@ fn trans_item(ccx: @crate_ctxt, item: ast::item) {
         // wouldn't make sense
         // So we initialize it here
         let selfptr = alloc_ty(bcx_top, rslt_ty);
+        // initialize fields to zero
+        let fields = ty::class_items_as_fields(bcx_top.tcx(),
+                                               local_def(item.id));
+        let mut bcx = bcx_top;
+        // Initialize fields to zero so init assignments can validly
+        // drop their LHS
+        for field in fields {
+           let ix = field_idx_strict(bcx.tcx(), ctor.span, field.ident,
+                                     fields);
+           bcx = zero_alloca(bcx, GEPi(bcx, selfptr, [0, ix]),
+                                       field.mt.ty);
+        }
+
+        // note we don't want to take *or* drop self.
         fcx.llself = some({v: selfptr, t: rslt_ty});
 
         // Translate the body of the ctor
-        let mut bcx = trans_block(bcx_top, ctor.node.body, ignore);
+        bcx = trans_block(bcx_top, ctor.node.body, ignore);
         let lval_res = {bcx: bcx, val: selfptr, kind: owned};
         // Generate the return expression
         bcx = store_temp_expr(bcx, INIT, fcx.llretptr, lval_res,

src/rustc/middle/trans/common.rs

@@ -884,6 +884,16 @@ fn node_id_type_params(bcx: block, id: ast::node_id) -> [ty::t] {
     }
 }
 
+fn field_idx_strict(cx: ty::ctxt, sp: span, ident: ast::ident,
+                    fields: [ty::field])
+    -> int {
+    alt ty::field_idx(ident, fields) {
+            none { cx.sess.span_bug(sp, #fmt("base expr doesn't appear to \
+                     have a field named %s", ident)); }
+            some(i) { i as int }
+        }
+}
+
 //
 // Local Variables:
 // mode: rust

src/rustc/middle/trans/shape.rs

@@ -57,7 +57,6 @@ const shape_stack_fn: u8 = 26u8;
 const shape_bare_fn: u8 = 27u8;
 const shape_tydesc: u8 = 28u8;
 const shape_send_tydesc: u8 = 29u8;
-const shape_class: u8 = 30u8;
 const shape_rptr: u8 = 31u8;
 
 fn hash_res_info(ri: res_info) -> uint {
@@ -370,7 +369,15 @@ fn shape_of(ccx: @crate_ctxt, t: ty::t, ty_param_map: [uint]) -> [u8] {
         s
       }
       ty::ty_iface(_, _) { [shape_box_fn] }
-      ty::ty_class(_, _) { [shape_class] }
+      ty::ty_class(did, _) {
+        // same as records
+        let mut s = [shape_struct], sub = [];
+        for f:field in ty::class_items_as_fields(ccx.tcx, did) {
+            sub += shape_of(ccx, f.mt.ty, ty_param_map);
+        }
+        add_substr(s, sub);
+        s
+      }
       ty::ty_rptr(_, tm) {
         let mut s = [shape_rptr];
         add_substr(s, shape_of(ccx, tm.ty, ty_param_map));

src/rustc/middle/ty.rs

@@ -41,7 +41,7 @@ export fm_var, fm_general, fm_rptr;
 export get_element_type;
 export is_binopable;
 export is_pred_ty;
-export lookup_class_fields;
+export lookup_class_field, lookup_class_fields;
 export lookup_class_method_by_name;
 export lookup_field_type;
 export lookup_item_type;
@@ -164,7 +164,8 @@ type mt = {ty: t, mutbl: ast::mutability};
 type field_ty = {
   ident: ident,
   id: def_id,
-  privacy: ast::privacy
+  privacy: ast::privacy,
+  mutability: ast::class_mutability
 };
 
 // Contains information needed to resolve types and (in the future) look up
@@ -864,6 +865,12 @@ fn type_needs_drop(cx: ctxt, ty: t) -> bool {
         for f in flds { if type_needs_drop(cx, f.mt.ty) { accum = true; } }
         accum
       }
+      ty_class(did,_) {
+          for f in ty::class_items_as_fields(cx, did)
+          { if type_needs_drop(cx, f.mt.ty) { accum = true; } }
+        accum
+      }
+
       ty_tup(elts) {
         for m in elts { if type_needs_drop(cx, m) { accum = true; } }
         accum
@@ -1956,11 +1963,6 @@ fn lookup_item_type(cx: ctxt, did: ast::def_id) -> ty_param_bounds_and_ty {
 // Look up a field ID, whether or not it's local
 fn lookup_field_type(tcx: ctxt, class_id: def_id, id: def_id) -> ty::t {
     if id.crate == ast::local_crate {
-            /*
-        alt items.find(tcx.items, id.node) {
-           some(ast_map::node_item({node: item_class(_,items,
-        }
-            */
         node_id_to_type(tcx, id.node)
     }
     else {
@@ -1999,6 +2001,15 @@ fn lookup_class_fields(cx: ctxt, did: ast::def_id) -> [field_ty] {
     }
 }
 
+fn lookup_class_field(cx: ctxt, parent: ast::def_id, field_id: ast::def_id)
+    -> field_ty {
+    alt vec::find(lookup_class_fields(cx, parent))
+                 {|f| f.id.node == field_id.node} {
+        some(t) { t }
+        none { cx.sess.bug("class ID not found in parent's fields"); }
+    }
+}
+
 fn lookup_public_fields(cx: ctxt, did: ast::def_id) -> [field_ty] {
     vec::filter(lookup_class_fields(cx, did), is_public)
 }
@@ -2050,9 +2061,9 @@ fn class_field_tys(items: [@class_item]) -> [field_ty] {
     let mut rslt = [];
     for it in items {
        alt it.node.decl {
-          instance_var(nm, _, _, id) {
+          instance_var(nm, _, cm, id) {
               rslt += [{ident: nm, id: ast_util::local_def(id),
-                          privacy: it.node.privacy}];
+                          privacy: it.node.privacy, mutability: cm}];
           }
           class_method(_) {
           }

src/rustc/syntax/parse/parser.rs

@@ -1684,8 +1684,8 @@ fn parse_let(p: parser) -> @ast::decl {
 fn parse_instance_var(p:parser) -> (ast::class_member, codemap::span) {
     let mut is_mutbl = ast::class_immutable;
     let lo = p.span.lo;
-    if eat_word(p, "mut") {
-            is_mutbl = ast::class_mutable;
+    if eat_word(p, "mut") || eat_word(p, "mutable") {
+        is_mutbl = ast::class_mutable;
     }
     if !is_plain_ident(p) {
         p.fatal("expecting ident");

src/test/auxiliary/cci_class_4.rs

@@ -12,9 +12,11 @@ class cat {
     }
   }
 
-  let how_hungry : int;
+  let mutable how_hungry : int;
+  let name : str;
 
-  new(in_x : uint, in_y : int) { meows = in_x; how_hungry = in_y; }
+  new(in_x : uint, in_y : int, in_name: str)
+    { meows = in_x; how_hungry = in_y; name = in_name; }
 
   fn speak() { meow(); }
 

src/test/compile-fail/mutable-class-fields-2.rs

@@ -0,0 +1,19 @@
+// error-pattern:assigning to immutable class field
+class cat {
+  priv {
+    let mutable meows : uint;
+  }
+
+  let how_hungry : int;
+
+  fn eat() {
+    how_hungry -= 5;
+  }
+
+  new(in_x : uint, in_y : int) { meows = in_x; how_hungry = in_y; }
+}
+
+fn main() {
+  let nyan : cat = cat(52u, 99);
+  nyan.eat();
+}