]> git.lizzy.rs Git - rust.git/commitdiff
Enforce privacy declarations for class fields and methods
authorTim Chevalier <chevalier@alum.wellesley.edu>
Mon, 26 Mar 2012 16:59:59 +0000 (09:59 -0700)
committerTim Chevalier <chevalier@alum.wellesley.edu>
Mon, 26 Mar 2012 17:00:33 +0000 (10:00 -0700)
12 files changed:
src/rustc/metadata/csearch.rs
src/rustc/metadata/decoder.rs
src/rustc/metadata/encoder.rs
src/rustc/middle/trans/base.rs
src/rustc/middle/ty.rs
src/rustc/middle/typeck.rs
src/rustc/syntax/ast_util.rs
src/test/compile-fail/assign-to-method.rs [new file with mode: 0644]
src/test/compile-fail/private-class-field.rs [new file with mode: 0644]
src/test/compile-fail/private-method.rs [new file with mode: 0644]
src/test/run-pass/private-class-field.rs [new file with mode: 0644]
src/test/run-pass/private-method.rs [new file with mode: 0644]

index e43b7aa7ff355241da6d66cb1a5ac64704ed798f..bada0931953234df1e0af020e11f693783066c59 100644 (file)
@@ -14,7 +14,6 @@
 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);
index 97a77dd71d3bbc6ef9c15bc9d26aa8ea1a8dd15e..c4860ebade7d8ac022dd74beb28365c7e80e867f 100644 (file)
@@ -17,7 +17,6 @@
 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
 }
 
-
-/* Take a node ID for a class, return a vector of the class's
-   field names/IDs */
-fn get_class_fields(cdata: cmd, id: ast::node_id) -> [ty::field_ty] {
-    get_class_members(cdata, id, 'g')
+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
-   method names/IDs */
-fn get_class_method_ids(cdata: cmd, id: ast::node_id) -> [ty::field_ty] {
-    get_class_members(cdata, id, 'h')
+/* '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, {|f| f == 'g' || f == 'j'})
 }
-*/
 
 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 }
index d648cc168bd6b4167ec36306152c2879e8033dd3..37d7cb600782d95f1cf939c12f35461b02f73c56 100644 (file)
@@ -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 add_to_index_(item: @item, ebml_w: ebml::writer,
         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);
index 8f87de76cbdc9f5104d22f5672d52b04304e5bf8..35a73a0a530f06b9f3dd25966945e0293422d549 100644 (file)
@@ -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 */ }
     }
index 6fc068bba277a44abe53732f191aa49576e3ce87..8fac6b536c001b36da4be97b48a6bace51e054f7 100644 (file)
@@ -45,6 +45,7 @@
 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;
 
 type mt = {ty: t, mutbl: ast::mutability};
 
-// Just use <field> 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(_) {
           }
index 0c3a7484d4238f4f0bc8bff42c2d9d4433b8b96c..20b705f07384bc6b89712acd26975e4d89caca1a 100644 (file)
@@ -11,7 +11,7 @@
 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 @@ fn store_methods<T>(tcx: ty::ctxt, id: ast::node_id,
           }
           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 @@ fn convert(tcx: ty::ctxt, it: @ast::item) {
               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 get_node(f: spanned<field>) -> field { f.node }
               // (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 get_node(f: spanned<field>) -> field { f.node }
               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); }
     }
 }
index 57dbfacbfbb346fe8af3fd50d3eb87d01959ff5b..308b13be88223deaed12db62c50d79e99cf233ba 100644 (file)
@@ -435,16 +435,29 @@ fn op_expr_callee_id(e: @expr) -> node_id { e.id - 1 }
     }
 }
 
-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 (file)
index 0000000..8a213b0
--- /dev/null
@@ -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 (file)
index 0000000..e7ba633
--- /dev/null
@@ -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 (file)
index 0000000..8b257c1
--- /dev/null
@@ -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 (file)
index 0000000..6d11ce2
--- /dev/null
@@ -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 (file)
index 0000000..6e95d3d
--- /dev/null
@@ -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();
+}