From c282810ab04a57f7d3f476e9628b6b82184b19b2 Mon Sep 17 00:00:00 2001 From: Tim Chevalier Date: Mon, 26 Mar 2012 09:59:59 -0700 Subject: [PATCH] Enforce privacy declarations for class fields and methods --- src/rustc/metadata/csearch.rs | 9 ------ src/rustc/metadata/decoder.rs | 31 +++++++++--------- src/rustc/metadata/encoder.rs | 31 ++++++++++++------ src/rustc/middle/trans/base.rs | 3 +- src/rustc/middle/ty.rs | 34 ++++++++++++++------ src/rustc/middle/typeck.rs | 24 +++++++++----- src/rustc/syntax/ast_util.rs | 21 +++++++++--- src/test/compile-fail/assign-to-method.rs | 16 +++++++++ src/test/compile-fail/private-class-field.rs | 24 ++++++++++++++ src/test/compile-fail/private-method.rs | 16 +++++++++ src/test/run-pass/private-class-field.rs | 15 +++++++++ src/test/run-pass/private-method.rs | 19 +++++++++++ 12 files changed, 185 insertions(+), 58 deletions(-) create mode 100644 src/test/compile-fail/assign-to-method.rs create mode 100644 src/test/compile-fail/private-class-field.rs create mode 100644 src/test/compile-fail/private-method.rs create mode 100644 src/test/run-pass/private-class-field.rs create mode 100644 src/test/run-pass/private-method.rs diff --git a/src/rustc/metadata/csearch.rs b/src/rustc/metadata/csearch.rs index e43b7aa7ff3..bada0931953 100644 --- a/src/rustc/metadata/csearch.rs +++ b/src/rustc/metadata/csearch.rs @@ -14,7 +14,6 @@ import std::map::hashmap; export get_symbol; export get_class_fields; export get_class_method; -// export get_class_method_ids; export get_field_type; export get_type_param_count; export lookup_defs; @@ -131,14 +130,6 @@ fn get_class_fields(tcx: ty::ctxt, def: ast::def_id) -> [ty::field_ty] { decoder::get_class_fields(cdata, def.node) } -/* -fn get_class_method_ids(tcx: ty::ctxt, def: ast::def_id) -> [ty::field_ty] { - let cstore = tcx.sess.cstore; - let cdata = cstore::get_crate_data(cstore, def.crate); - decoder::get_class_method_ids(cdata, def.node) -} -*/ - fn get_type(tcx: ty::ctxt, def: ast::def_id) -> ty::ty_param_bounds_and_ty { let cstore = tcx.sess.cstore; let cdata = cstore::get_crate_data(cstore, def.crate); diff --git a/src/rustc/metadata/decoder.rs b/src/rustc/metadata/decoder.rs index 97a77dd71d3..c4860ebade7 100644 --- a/src/rustc/metadata/decoder.rs +++ b/src/rustc/metadata/decoder.rs @@ -17,7 +17,6 @@ import middle::trans::common::maps; import util::ppaux::ty_to_str; export get_class_fields; -// export get_class_method_ids; export get_symbol; export get_enum_variants; export get_type; @@ -427,38 +426,38 @@ fn get_iface_methods(cdata: cmd, id: ast::node_id, tcx: ty::ctxt) // Helper function that gets either fields or methods fn get_class_members(cdata: cmd, id: ast::node_id, - family: char) -> [ty::field_ty] { + p: fn(char) -> bool) -> [ty::field_ty] { let data = cdata.data; let item = lookup_item(id, data); let mut result = []; ebml::tagged_docs(item, tag_item_field) {|an_item| - if item_family(an_item) == family { + let f = item_family(an_item); + if p(f) { let name = item_name(an_item); let did = class_member_id(an_item, cdata); - result += [{ident: name, id: did}]; + result += [{ident: name, id: did, privacy: + // This won't work for methods, argh + family_to_privacy(f)}]; } } result } +pure fn family_to_privacy(family: char) -> ast::privacy { + alt family { + 'g' { ast::pub } + _ { ast::priv } + } +} -/* Take a node ID for a class, return a vector of the class's - field names/IDs */ +/* 'g' for public field, 'j' for private field */ fn get_class_fields(cdata: cmd, id: ast::node_id) -> [ty::field_ty] { - get_class_members(cdata, id, 'g') + get_class_members(cdata, id, {|f| f == 'g' || f == 'j'}) } -/* -/* Take a node ID for a class, return a vector of the class's - method names/IDs */ -fn get_class_method_ids(cdata: cmd, id: ast::node_id) -> [ty::field_ty] { - get_class_members(cdata, id, 'h') -} -*/ - fn family_has_type_params(fam_ch: char) -> bool { alt check fam_ch { - 'c' | 'T' | 'm' | 'n' | 'g' | 'h' { false } + 'c' | 'T' | 'm' | 'n' | 'g' | 'h' | 'j' { false } 'f' | 'u' | 'p' | 'F' | 'U' | 'P' | 'y' | 't' | 'v' | 'i' | 'I' | 'C' | 'a' { true } diff --git a/src/rustc/metadata/encoder.rs b/src/rustc/metadata/encoder.rs index d648cc168bd..37d7cb60078 100644 --- a/src/rustc/metadata/encoder.rs +++ b/src/rustc/metadata/encoder.rs @@ -353,6 +353,11 @@ fn encode_info_for_mod(ecx: @encode_ctxt, ebml_w: ebml::writer, md: _mod, ebml_w.end_tag(); } +fn encode_privacy(ebml_w: ebml::writer, privacy: privacy) { + encode_family(ebml_w, alt privacy { + pub { 'g' } priv { 'j' }}); +} + /* Returns an index of items in this class */ fn encode_info_for_class(ecx: @encode_ctxt, ebml_w: ebml::writer, id: node_id, path: ast_map::path, @@ -369,7 +374,7 @@ fn encode_info_for_class(ecx: @encode_ctxt, ebml_w: ebml::writer, *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_family(ebml_w, 'g'); + 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)); @@ -564,19 +569,25 @@ fn encode_info_for_item(ecx: @encode_ctxt, ebml_w: ebml::writer, item: @item, let (fs,ms) = ast_util::split_class_items(items); for f in fs { ebml_w.start_tag(tag_item_field); - encode_family(ebml_w, 'g'); + encode_privacy(ebml_w, f.privacy); encode_name(ebml_w, f.ident); encode_def_id(ebml_w, local_def(f.id)); ebml_w.end_tag(); } - for m in ms { - ebml_w.start_tag(tag_item_method); - #debug("Writing %s %d", m.ident, m.id); - encode_family(ebml_w, purity_fn_family(m.decl.purity)); - encode_name(ebml_w, m.ident); - encode_type(ecx, ebml_w, node_id_to_type(tcx, m.id)); - encode_def_id(ebml_w, local_def(m.id)); - ebml_w.end_tag(); + for mt in ms { + alt mt.privacy { + priv { /* do nothing */ } + pub { + let m = mt.meth; + ebml_w.start_tag(tag_item_method); + #debug("Writing %s %d", m.ident, m.id); + encode_family(ebml_w, purity_fn_family(m.decl.purity)); + encode_name(ebml_w, m.ident); + encode_type(ecx, ebml_w, node_id_to_type(tcx, m.id)); + encode_def_id(ebml_w, local_def(m.id)); + ebml_w.end_tag(); + } + } } /* Each class has its own index -- encode it */ let bkts = create_index(idx, hash_node_id); diff --git a/src/rustc/middle/trans/base.rs b/src/rustc/middle/trans/base.rs index 8f87de76cbd..35a73a0a530 100644 --- a/src/rustc/middle/trans/base.rs +++ b/src/rustc/middle/trans/base.rs @@ -4271,7 +4271,8 @@ fn trans_item(ccx: @crate_ctxt, item: ast::item) { // Translate methods let (_, ms) = ast_util::split_class_items(items); - impl::trans_impl(ccx, *path, item.ident, ms, tps); + impl::trans_impl(ccx, *path, item.ident, + vec::map(ms, {|m| m.meth}), tps); } _ {/* fall through */ } } diff --git a/src/rustc/middle/ty.rs b/src/rustc/middle/ty.rs index 6fc068bba27..8fac6b536c0 100644 --- a/src/rustc/middle/ty.rs +++ b/src/rustc/middle/ty.rs @@ -45,6 +45,7 @@ export lookup_class_fields; export lookup_class_method_by_name; export lookup_field_type; export lookup_item_type; +export lookup_public_fields; export method; export method_idx; export mk_class; @@ -160,10 +161,10 @@ type constr_table = hashmap; type mt = {ty: t, mutbl: ast::mutability}; -// Just use for class fields? type field_ty = { ident: ident, id: def_id, + privacy: ast::privacy }; // Contains information needed to resolve types and (in the future) look up @@ -1996,14 +1997,26 @@ fn lookup_class_fields(cx: ctxt, did: ast::def_id) -> [field_ty] { } } +fn lookup_public_fields(cx: ctxt, did: ast::def_id) -> [field_ty] { + vec::filter(lookup_class_fields(cx, did), is_public) +} + +pure fn is_public(f: field_ty) -> bool { + alt f.privacy { + pub { true } + priv { false } + } +} + // Look up the list of method names and IDs for a given class // Fails if the id is not bound to a class. fn lookup_class_method_ids(cx: ctxt, did: ast::def_id) - : is_local(did) -> [{name: ident, id: node_id}] { + : is_local(did) -> [{name: ident, id: node_id, privacy: privacy}] { alt cx.items.find(did.node) { some(ast_map::node_item(@{node: item_class(_,items,_), _}, _)) { let (_,ms) = split_class_items(items); - vec::map(ms, {|m| {name: m.ident, id: m.id}}) + vec::map(ms, {|m| {name: m.meth.ident, id: m.meth.id, + privacy: m.privacy}}) } _ { cx.sess.bug("lookup_class_method_ids: id not bound to a class"); @@ -2012,19 +2025,19 @@ fn lookup_class_method_ids(cx: ctxt, did: ast::def_id) } /* Given a class def_id and a method name, return the method's - def_id. Needed so we can do static dispatch for methods */ + def_id. Needed so we can do static dispatch for methods + Fails if the requested method is private */ fn lookup_class_method_by_name(cx:ctxt, did: ast::def_id, name: ident, - sp: span) -> - def_id { + sp: span) -> def_id { if check is_local(did) { let ms = lookup_class_method_ids(cx, did); for m in ms { - if m.name == name { + if m.name == name && m.privacy == ast::pub { ret ast_util::local_def(m.id); } } - cx.sess.span_fatal(sp, #fmt("Class doesn't have a method named %s", - name)); + cx.sess.span_fatal(sp, #fmt("Class doesn't have a public method \ + named %s", name)); } else { csearch::get_class_method(cx.sess.cstore, did, name) @@ -2036,7 +2049,8 @@ fn class_field_tys(items: [@class_item]) -> [field_ty] { for it in items { alt it.node.decl { instance_var(nm, _, _, id) { - rslt += [{ident: nm, id: ast_util::local_def(id)}]; + rslt += [{ident: nm, id: ast_util::local_def(id), + privacy: it.node.privacy}]; } class_method(_) { } diff --git a/src/rustc/middle/typeck.rs b/src/rustc/middle/typeck.rs index 0c3a7484d42..20b705f0738 100644 --- a/src/rustc/middle/typeck.rs +++ b/src/rustc/middle/typeck.rs @@ -11,7 +11,7 @@ import pat_util::*; import middle::ty; import middle::ty::{node_id_to_type, arg, block_ty, expr_ty, field, node_type_table, mk_nil, - ty_param_bounds_and_ty, lookup_class_fields}; + ty_param_bounds_and_ty, lookup_public_fields}; import util::ppaux::ty_to_str; import std::smallintmap; import std::map::{hashmap, int_hash}; @@ -934,7 +934,9 @@ mod collect { } ast_map::node_item(@{node: ast::item_class(_,its,_), _}, _) { let (_,ms) = split_class_items(its); - store_methods::<@ast::method>(tcx, id, ms, {|m| + // Handling all methods here + let ps = ast_util::ignore_privacy(ms); + store_methods::<@ast::method>(tcx, id, ps, {|m| ty_of_method(tcx, m_collect, m)}); } } @@ -1089,10 +1091,13 @@ mod collect { for f in fields { convert_class_item(tcx, f); } + // The selfty is just the class type let selfty = ty::mk_class(tcx, local_def(it.id), mk_ty_params(tcx, tps).params); - // The selfty is just the class type - convert_methods(tcx, methods, @[], some(selfty)); + // Need to convert all methods so we can check internal + // references to private methods + convert_methods(tcx, ast_util::ignore_privacy(methods), @[], + some(selfty)); } _ { // This call populates the type cache with the converted type @@ -3095,7 +3100,11 @@ fn check_expr_with_unifier(fcx: @fn_ctxt, expr: @ast::expr, unify: unifier, // (1) verify that the class id actually has a field called // field #debug("class named %s", ty_to_str(tcx, base_t)); - let cls_items = lookup_class_fields(tcx, base_id); + /* + This is an external reference, so only consider public + fields + */ + let cls_items = lookup_public_fields(tcx, base_id); #debug("cls_items: %?", cls_items); alt lookup_field_ty(tcx, base_id, cls_items, field) { some(field_ty) { @@ -3119,7 +3128,7 @@ fn check_expr_with_unifier(fcx: @fn_ctxt, expr: @ast::expr, unify: unifier, none { let t_err = resolve_type_vars_if_possible(fcx, expr_t); let msg = #fmt["attempted access of field %s on type %s, but \ - no field or method with that name was found", + no public field or method with that name was found", field, ty_to_str(tcx, t_err)]; tcx.sess.span_err(expr.span, msg); // NB: Adding a bogus type to allow typechecking to continue @@ -3613,9 +3622,8 @@ fn class_types(ccx: @crate_ctxt, members: [@ast::class_item]) -> class_map { fn check_class_member(ccx: @crate_ctxt, cm: ast::class_member) { alt cm { - ast::instance_var(_,t,_,_) { // ??? Not sure + ast::instance_var(_,t,_,_) { } - // not right yet -- need a scope ast::class_method(m) { check_method(ccx, m); } } } diff --git a/src/rustc/syntax/ast_util.rs b/src/rustc/syntax/ast_util.rs index 57dbfacbfbb..308b13be882 100644 --- a/src/rustc/syntax/ast_util.rs +++ b/src/rustc/syntax/ast_util.rs @@ -435,16 +435,29 @@ pure fn class_item_ident(ci: @class_item) -> ident { } } -type ivar = {ident: ident, ty: @ty, cm: class_mutability, id: node_id}; +type ivar = {ident: ident, ty: @ty, cm: class_mutability, + id: node_id, privacy: privacy}; -fn split_class_items(cs: [@class_item]) -> ([ivar], [@method]) { +type cmethod = {privacy: privacy, meth: @method}; + +fn public_methods(cms: [cmethod]) -> [@method] { + vec::filter_map(cms, {|cm| alt cm.privacy { + pub { some(cm.meth) } + _ { none }}}) +} + +fn ignore_privacy(cms: [cmethod]) -> [@method] { + vec::map(cms, {|cm| cm.meth}) +} + +fn split_class_items(cs: [@class_item]) -> ([ivar], [cmethod]) { let mut vs = [], ms = []; for c in cs { alt c.node.decl { instance_var(i, t, cm, id) { - vs += [{ident: i, ty: t, cm: cm, id: id}]; + vs += [{ident: i, ty: t, cm: cm, id: id, privacy: c.node.privacy}]; } - class_method(m) { ms += [m]; } + class_method(m) { ms += [{privacy: c.node.privacy, meth: m}]; } } } (vs, ms) diff --git a/src/test/compile-fail/assign-to-method.rs b/src/test/compile-fail/assign-to-method.rs new file mode 100644 index 00000000000..8a213b08497 --- /dev/null +++ b/src/test/compile-fail/assign-to-method.rs @@ -0,0 +1,16 @@ +// error-pattern:assigning to immutable field +class cat { + priv { + let mutable meows : uint; + } + + let how_hungry : int; + + fn speak() { meows += 1u; } + new(in_x : uint, in_y : int) { meows = in_x; how_hungry = in_y; } +} + +fn main() { + let nyan : cat = cat(52u, 99); + nyan.speak = fn@() { log(error, "meow"); }; +} diff --git a/src/test/compile-fail/private-class-field.rs b/src/test/compile-fail/private-class-field.rs new file mode 100644 index 00000000000..e7ba6339ea7 --- /dev/null +++ b/src/test/compile-fail/private-class-field.rs @@ -0,0 +1,24 @@ +// error-pattern:no public field or method with that name +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); + assert (nyan.meows == 52u); +} +/* + other tests: + not ok to refer to a private method outside a class + ok to refer to private method within a class + can't assign to a non-mutable var + can't assign to a method + + all the same tests, cross-crate + */ \ No newline at end of file diff --git a/src/test/compile-fail/private-method.rs b/src/test/compile-fail/private-method.rs new file mode 100644 index 00000000000..8b257c11cd9 --- /dev/null +++ b/src/test/compile-fail/private-method.rs @@ -0,0 +1,16 @@ +// error-pattern:Class doesn't have a public method named nap +class cat { + priv { + let mutable meows : uint; + fn nap() { uint::range(1u, 10000u) {|_i|}} + } + + 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.nap(); +} diff --git a/src/test/run-pass/private-class-field.rs b/src/test/run-pass/private-class-field.rs new file mode 100644 index 00000000000..6d11ce2c3da --- /dev/null +++ b/src/test/run-pass/private-class-field.rs @@ -0,0 +1,15 @@ +class cat { + priv { + let mutable meows : uint; + } + + let how_hungry : int; + + fn meow_count() -> uint { meows } + new(in_x : uint, in_y : int) { meows = in_x; how_hungry = in_y; } +} + +fn main() { + let nyan : cat = cat(52u, 99); + assert (nyan.meow_count() == 52u); +} diff --git a/src/test/run-pass/private-method.rs b/src/test/run-pass/private-method.rs new file mode 100644 index 00000000000..6e95d3d314f --- /dev/null +++ b/src/test/run-pass/private-method.rs @@ -0,0 +1,19 @@ +class cat { + priv { + let mutable meows : uint; + fn nap() { uint::range(1u, 10u) {|_i|}} + } + + let how_hungry : int; + + fn play() { + meows += 1u; + nap(); + } + new(in_x : uint, in_y : int) { meows = in_x; how_hungry = in_y; } +} + +fn main() { + let nyan : cat = cat(52u, 99); + nyan.play(); +}