From edb747ceedcfec2eb9f698b0e11b7e94d9174f33 Mon Sep 17 00:00:00 2001 From: Tim Chevalier Date: Tue, 27 Mar 2012 22:08:48 -0700 Subject: [PATCH] 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). --- src/rustc/metadata/common.rs | 1 + src/rustc/metadata/decoder.rs | 16 ++++- src/rustc/metadata/encoder.rs | 11 +++- src/rustc/middle/mutbl.rs | 60 +++++++++++++++---- src/rustc/middle/trans/base.rs | 24 +++++--- src/rustc/middle/trans/common.rs | 10 ++++ src/rustc/middle/trans/shape.rs | 11 +++- src/rustc/middle/ty.rs | 29 ++++++--- src/rustc/syntax/parse/parser.rs | 4 +- src/test/auxiliary/cci_class_4.rs | 6 +- .../compile-fail/mutable-class-fields-2.rs | 19 ++++++ src/test/compile-fail/mutable-class-fields.rs | 15 +++++ src/test/run-pass/class-str-field.rs | 11 ++++ src/test/run-pass/classes-cross-crate.rs | 2 +- src/test/run-pass/classes.rs | 8 ++- 15 files changed, 187 insertions(+), 40 deletions(-) create mode 100644 src/test/compile-fail/mutable-class-fields-2.rs create mode 100644 src/test/compile-fail/mutable-class-fields.rs create mode 100644 src/test/run-pass/class-str-field.rs diff --git a/src/rustc/metadata/common.rs b/src/rustc/metadata/common.rs index a446d5ba90b..0146421c689 100644 --- a/src/rustc/metadata/common.rs +++ b/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 diff --git a/src/rustc/metadata/decoder.rs b/src/rustc/metadata/decoder.rs index c4860ebade7..0b67a16c072 100644 --- a/src/rustc/metadata/decoder.rs +++ b/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 { 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 diff --git a/src/rustc/metadata/encoder.rs b/src/rustc/metadata/encoder.rs index a573d0214dc..2c67b907923 100644 --- a/src/rustc/metadata/encoder.rs +++ b/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 = {val: T, pos: uint}; fn encode_enum_variant_paths(ebml_w: ebml::writer, variants: [variant], @@ -370,7 +377,7 @@ 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); @@ -378,7 +385,7 @@ fn encode_info_for_class(ecx: @encode_ctxt, ebml_w: ebml::writer, 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(); } diff --git a/src/rustc/middle/mutbl.rs b/src/rustc/middle/mutbl.rs index 092afcdbedd..8bebb3b5e68 100644 --- a/src/rustc/middle/mutbl.rs +++ b/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; -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 { 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 { } 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 } } } diff --git a/src/rustc/middle/trans/base.rs b/src/rustc/middle/trans/base.rs index f8baf6b93aa..82eec9b8bd6 100644 --- a/src/rustc/middle/trans/base.rs +++ b/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, diff --git a/src/rustc/middle/trans/common.rs b/src/rustc/middle/trans/common.rs index 5dd282c8df3..fe4b58dc981 100644 --- a/src/rustc/middle/trans/common.rs +++ b/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 diff --git a/src/rustc/middle/trans/shape.rs b/src/rustc/middle/trans/shape.rs index 9a90d1e8270..3786dbd4e9e 100644 --- a/src/rustc/middle/trans/shape.rs +++ b/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)); diff --git a/src/rustc/middle/ty.rs b/src/rustc/middle/ty.rs index 6ef1e2722a4..c642a411d56 100644 --- a/src/rustc/middle/ty.rs +++ b/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(_) { } diff --git a/src/rustc/syntax/parse/parser.rs b/src/rustc/syntax/parse/parser.rs index a898ee840be..3ca2e6a588b 100644 --- a/src/rustc/syntax/parse/parser.rs +++ b/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"); diff --git a/src/test/auxiliary/cci_class_4.rs b/src/test/auxiliary/cci_class_4.rs index 4bebf081e23..e44a90e5a5c 100644 --- a/src/test/auxiliary/cci_class_4.rs +++ b/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(); } diff --git a/src/test/compile-fail/mutable-class-fields-2.rs b/src/test/compile-fail/mutable-class-fields-2.rs new file mode 100644 index 00000000000..edfb5a92058 --- /dev/null +++ b/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(); +} diff --git a/src/test/compile-fail/mutable-class-fields.rs b/src/test/compile-fail/mutable-class-fields.rs new file mode 100644 index 00000000000..b345c0c80f1 --- /dev/null +++ b/src/test/compile-fail/mutable-class-fields.rs @@ -0,0 +1,15 @@ +// error-pattern:assigning to immutable field +class cat { + priv { + let mutable meows : uint; + } + + let how_hungry : int; + + new(in_x : uint, in_y : int) { meows = in_x; how_hungry = in_y; } +} + +fn main() { + let nyan : cat = cat(52u, 99); + nyan.how_hungry = 0; +} diff --git a/src/test/run-pass/class-str-field.rs b/src/test/run-pass/class-str-field.rs new file mode 100644 index 00000000000..831006a7dfd --- /dev/null +++ b/src/test/run-pass/class-str-field.rs @@ -0,0 +1,11 @@ +class cat { + + let name : str; + + new(in_name: str) + { name = in_name; } +} + +fn main() { + let nyan = cat("nyan"); +} \ No newline at end of file diff --git a/src/test/run-pass/classes-cross-crate.rs b/src/test/run-pass/classes-cross-crate.rs index b4f46c51305..3c2efa501e4 100644 --- a/src/test/run-pass/classes-cross-crate.rs +++ b/src/test/run-pass/classes-cross-crate.rs @@ -4,7 +4,7 @@ use cci_class_4; import cci_class_4::kitties::*; fn main() { - let nyan = cat(0u, 2); + let nyan = cat(0u, 2, "nyan"); nyan.eat(); assert(!nyan.eat()); uint::range(1u, 10u, {|_i| nyan.speak(); }); diff --git a/src/test/run-pass/classes.rs b/src/test/run-pass/classes.rs index 0117519089a..32add1b606d 100644 --- a/src/test/run-pass/classes.rs +++ b/src/test/run-pass/classes.rs @@ -10,9 +10,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(); } @@ -30,7 +32,7 @@ class cat { } fn main() { - let nyan = cat(0u, 2); + let nyan = cat(0u, 2, "nyan"); nyan.eat(); assert(!nyan.eat()); uint::range(1u, 10u, {|_i| nyan.speak(); });