Enforce privacy declarations for class fields and methods

7 files changed, 95 insertions, 58 deletions

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
 }
 
-
-/* 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 }
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<ast::node_id, [constr]>;
 
 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(_) {
           }
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)