Correctly encode item visibility in metadata

This fixes private statics and functions from being usable cross-crates

Author: alexcrichton

src/librustc/driver/driver.rs

@@ -195,6 +195,7 @@ pub fn phase_2_configure_and_expand(sess: Session,
 
 pub struct CrateAnalysis {
     exp_map2: middle::resolve::ExportMap2,
+    exported_items: @mut middle::resolve::ExportedItems,
     ty_cx: ty::ctxt,
     maps: astencode::Maps,
     reachable: @mut HashSet<ast::NodeId>
@@ -223,7 +224,8 @@ pub fn phase_3_run_analysis_passes(sess: Session,
     let middle::resolve::CrateMap {
         def_map: def_map,
         exp_map2: exp_map2,
-        trait_map: trait_map
+        trait_map: trait_map,
+        exported_items: exported_items
     } =
         time(time_passes, ~"resolution", ||
              middle::resolve::resolve_crate(sess, lang_items, crate));
@@ -298,6 +300,7 @@ pub fn phase_3_run_analysis_passes(sess: Session,
 
     CrateAnalysis {
         exp_map2: exp_map2,
+        exported_items: exported_items,
         ty_cx: ty_cx,
         maps: astencode::Maps {
             root_map: root_map,

src/librustc/metadata/csearch.rs

@@ -27,7 +27,8 @@ use syntax::diagnostic::expect;
 pub struct StaticMethodInfo {
     ident: ast::ident,
     def_id: ast::def_id,
-    purity: ast::purity
+    purity: ast::purity,
+    vis: ast::visibility,
 }
 
 pub fn get_symbol(cstore: @mut cstore::CStore, def: ast::def_id) -> ~str {

src/librustc/metadata/decoder.rs

@@ -525,7 +525,8 @@ impl<'self> EachItemContext<'self> {
     fn process_item_and_pop_name(&mut self,
                                  doc: ebml::Doc,
                                  def_id: ast::def_id,
-                                 old_len: uint)
+                                 old_len: uint,
+                                 vis: ast::visibility)
                                  -> bool {
         let def_like = item_to_def_like(doc, def_id, self.cdata.cnum);
         match def_like {
@@ -544,8 +545,6 @@ impl<'self> EachItemContext<'self> {
             }
         }
 
-        let vis = item_visibility(doc);
-
         let mut continue = (self.callback)(*self.path_builder, def_like, vis);
 
         let family = item_family(doc);
@@ -634,9 +633,12 @@ impl<'self> EachItemContext<'self> {
                         self.push_name(token::ident_to_str(&child_name));
 
                     // Process this item.
+
+                    let vis = item_visibility(child_item_doc);
                     continue = self.process_item_and_pop_name(child_item_doc,
                                                               child_def_id,
-                                                              old_len);
+                                                              old_len,
+                                                              vis);
                 }
             }
             continue
@@ -682,12 +684,13 @@ impl<'self> EachItemContext<'self> {
 
             // Get the item.
             match maybe_find_item(def_id.node, other_crates_items) {
-                None => {}
+                None => { self.pop_name(old_len); }
                 Some(reexported_item_doc) => {
                     continue = self.process_item_and_pop_name(
                         reexported_item_doc,
                         def_id,
-                        old_len);
+                        old_len,
+                        ast::public);
                 }
             }
 
@@ -1007,7 +1010,8 @@ pub fn get_static_methods_if_impl(intr: @ident_interner,
                 static_impl_methods.push(StaticMethodInfo {
                     ident: item_name(intr, impl_method_doc),
                     def_id: item_def_id(impl_method_doc, cdata),
-                    purity: purity
+                    purity: purity,
+                    vis: item_visibility(impl_method_doc),
                 });
             }
             _ => {}

src/librustc/metadata/encoder.rs

@@ -16,9 +16,9 @@ use metadata::cstore;
 use metadata::decoder;
 use metadata::tyencode;
 use middle::ty::{node_id_to_type, lookup_item_type};
+use middle::astencode;
 use middle::ty;
 use middle::typeck;
-use middle::astencode;
 use middle;
 
 use std::hash::HashUtil;
@@ -58,6 +58,7 @@ pub struct EncodeParams<'self> {
     diag: @mut span_handler,
     tcx: ty::ctxt,
     reexports2: middle::resolve::ExportMap2,
+    exported_items: @mut middle::resolve::ExportedItems,
     item_symbols: &'self HashMap<ast::NodeId, ~str>,
     discrim_symbols: &'self HashMap<ast::NodeId, @str>,
     link_meta: &'self LinkMeta,
@@ -86,6 +87,7 @@ pub struct EncodeContext<'self> {
     tcx: ty::ctxt,
     stats: @mut Stats,
     reexports2: middle::resolve::ExportMap2,
+    exported_items: @mut middle::resolve::ExportedItems,
     item_symbols: &'self HashMap<ast::NodeId, ~str>,
     discrim_symbols: &'self HashMap<ast::NodeId, @str>,
     link_meta: &'self LinkMeta,
@@ -808,7 +810,8 @@ fn encode_info_for_item(ecx: &EncodeContext,
                         ebml_w: &mut writer::Encoder,
                         item: @item,
                         index: @mut ~[entry<i64>],
-                        path: &[ast_map::path_elt]) {
+                        path: &[ast_map::path_elt],
+                        vis: ast::visibility) {
     let tcx = ecx.tcx;
 
     fn add_to_index_(item: @item, ebml_w: &writer::Encoder,
@@ -836,6 +839,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
         encode_name(ecx, ebml_w, item.ident);
         encode_path(ecx, ebml_w, path, ast_map::path_name(item.ident));
         (ecx.encode_inlined_item)(ecx, ebml_w, path, ii_item(item));
+        encode_visibility(ebml_w, vis);
         ebml_w.end_tag();
       }
       item_fn(_, purity, _, ref generics, _) => {
@@ -853,6 +857,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
         } else {
             encode_symbol(ecx, ebml_w, item.id);
         }
+        encode_visibility(ebml_w, vis);
         ebml_w.end_tag();
       }
       item_mod(ref m) => {
@@ -879,7 +884,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
             ebml_w.wr_str(def_to_str(local_def(foreign_item.id)));
             ebml_w.end_tag();
         }
-
+        encode_visibility(ebml_w, vis);
         ebml_w.end_tag();
       }
       item_ty(*) => {
@@ -891,6 +896,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
         encode_name(ecx, ebml_w, item.ident);
         encode_path(ecx, ebml_w, path, ast_map::path_name(item.ident));
         encode_region_param(ecx, ebml_w, item);
+        encode_visibility(ebml_w, vis);
         ebml_w.end_tag();
       }
       item_enum(ref enum_definition, ref generics) => {
@@ -907,6 +913,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
         (ecx.encode_inlined_item)(ecx, ebml_w, path, ii_item(item));
         encode_path(ecx, ebml_w, path, ast_map::path_name(item.ident));
         encode_region_param(ecx, ebml_w, item);
+        encode_visibility(ebml_w, vis);
         ebml_w.end_tag();
 
         encode_enum_variant_info(ecx,
@@ -938,6 +945,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
         encode_attributes(ebml_w, item.attrs);
         encode_path(ecx, ebml_w, path, ast_map::path_name(item.ident));
         encode_region_param(ecx, ebml_w, item);
+        encode_visibility(ebml_w, vis);
 
         /* Encode def_ids for each field and method
          for methods, write all the stuff get_trait_method
@@ -1187,7 +1195,12 @@ fn my_visit_item(i:@item, items: ast_map::map, ebml_w:&writer::Encoder,
             let mut ebml_w = ebml_w.clone();
             // See above
             let ecx : &EncodeContext = unsafe { cast::transmute(ecx_ptr) };
-            encode_info_for_item(ecx, &mut ebml_w, i, index, *pt);
+            let vis = if ecx.exported_items.contains(&i.id) {
+                ast::public
+            } else {
+                ast::inherited
+            };
+            encode_info_for_item(ecx, &mut ebml_w, i, index, *pt, vis);
         }
         _ => fail!("bad item")
     }
@@ -1592,6 +1605,7 @@ pub fn encode_metadata(parms: EncodeParams, crate: &Crate) -> ~[u8] {
         diag,
         tcx,
         reexports2,
+        exported_items,
         discrim_symbols,
         cstore,
         encode_inlined_item,
@@ -1606,6 +1620,7 @@ pub fn encode_metadata(parms: EncodeParams, crate: &Crate) -> ~[u8] {
         tcx: tcx,
         stats: stats,
         reexports2: reexports2,
+        exported_items: exported_items,
         item_symbols: item_symbols,
         discrim_symbols: discrim_symbols,
         link_meta: link_meta,

src/librustc/metadata/filesearch.rs

@@ -128,7 +128,7 @@ fn make_target_lib_path(sysroot: &Path,
     sysroot.push_rel(&relative_target_lib_path(target_triple))
 }
 
-fn get_or_default_sysroot() -> Path {
+pub fn get_or_default_sysroot() -> Path {
     match os::self_exe_path() {
       option::Some(ref p) => (*p).pop(),
       option::None => fail!("can't determine value for sysroot")

src/librustc/middle/privacy.rs

@@ -261,12 +261,12 @@ impl PrivacyVisitor {
                                           fmt!("function `%s` is private",
                                                token::ident_to_str(path.idents.last())));
                     }
-                } else if csearch::get_item_visibility(self.tcx.sess.cstore,
-                                                       def_id) != public {
-                    self.tcx.sess.span_err(span,
-                                      fmt!("function `%s` is private",
-                                           token::ident_to_str(path.idents.last())));
                 }
+                // If this is a function from a non-local crate, then the
+                // privacy check is enforced during resolve. All public items
+                // will be tagged as such in the crate metadata and then usage
+                // of the private items will be blocked during resolve. Hence,
+                // if this isn't from the local crate, nothing to check.
             }
             _ => {}
         }

src/librustc/middle/resolve.rs

@@ -65,6 +65,11 @@ pub struct Export2 {
     reexport: bool,     // Whether this is a reexport.
 }
 
+// This set is a set of all item nodes which can be used by external crates if
+// we're building a library. The necessary qualifications for this are that all
+// items leading down to the current item (excluding an `impl`) must be `pub`.
+pub type ExportedItems = HashSet<NodeId>;
+
 #[deriving(Eq)]
 pub enum PatternBindingMode {
     RefutableMode,
@@ -818,6 +823,7 @@ pub fn Resolver(session: Session,
 
         xray_context: NoXray,
         current_trait_refs: None,
+        path_all_public: true,
 
         self_ident: special_idents::self_,
         type_self_ident: special_idents::type_self,
@@ -830,6 +836,7 @@ pub fn Resolver(session: Session,
         export_map2: @mut HashMap::new(),
         trait_map: HashMap::new(),
         used_imports: HashSet::new(),
+        exported_items: @mut HashSet::new(),
 
         emit_errors: true,
         intr: session.intr()
@@ -874,6 +881,13 @@ pub struct Resolver {
     // The trait that the current context can refer to.
     current_trait_refs: Option<~[def_id]>,
 
+    // A set of all items which are re-exported to be used across crates
+    exported_items: @mut ExportedItems,
+
+    // When determinining the list of exported items, this is relevant for
+    // determining if `pub` uses should be re-exported across crates
+    path_all_public: bool,
+
     // The ident for the keyword "self".
     self_ident: ident,
     // The ident for the non-keyword "Self".
@@ -1780,8 +1794,8 @@ impl Resolver {
                 |path_string, def_like, visibility| {
 
             debug!("(building reduced graph for external crate) found path \
-                        entry: %s (%?)",
-                    path_string, def_like);
+                        entry: %s (%?, %?)",
+                    path_string, def_like, visibility);
 
             let mut pieces: ~[&str] = path_string.split_str_iter("::").collect();
             let final_ident_str = pieces.pop();
@@ -1927,8 +1941,10 @@ impl Resolver {
                                         let def = def_fn(
                                             static_method_info.def_id,
                                             static_method_info.purity);
+                                        let p = visibility_to_privacy(
+                                            static_method_info.vis);
                                         method_name_bindings.define_value(
-                                            Public, def, dummy_sp());
+                                            p, def, dummy_sp());
                                     }
                                 }
 
@@ -3252,14 +3268,15 @@ impl Resolver {
         match (namebindings.def_for_namespace(ns),
                namebindings.privacy_for_namespace(ns)) {
             (Some(d), Some(Public)) => {
+                let def = def_id_of_def(d);
                 debug!("(computing exports) YES: %s '%s' => %?",
                        if reexport { ~"reexport" } else { ~"export"},
                        self.session.str_of(ident),
-                       def_id_of_def(d));
+                       def);
                 exports2.push(Export2 {
                     reexport: reexport,
                     name: self.session.str_of(ident),
-                    def_id: def_id_of_def(d)
+                    def_id: def,
                 });
             }
             (Some(_), Some(privacy)) => {
@@ -3511,6 +3528,13 @@ impl Resolver {
             self.xray_context = Xray;
         }
 
+        let orig_path_all_public = self.path_all_public;
+        self.path_all_public = self.path_all_public && item.vis == ast::public;
+
+        if self.path_all_public {
+            self.exported_items.insert(item.id);
+        }
+
         match item.node {
 
             // enum item: resolve all the variants' discrs,
@@ -3550,6 +3574,10 @@ impl Resolver {
                       ref implemented_traits,
                       ref self_type,
                       ref methods) => {
+                self.path_all_public = orig_path_all_public;
+                if self.path_all_public {
+                    self.exported_items.insert(item.id);
+                }
                 self.resolve_implementation(item.id,
                                             generics,
                                             implemented_traits,
@@ -3634,6 +3662,11 @@ impl Resolver {
             }
 
             item_foreign_mod(ref foreign_module) => {
+                self.path_all_public = orig_path_all_public;
+                if self.path_all_public {
+                    self.exported_items.insert(item.id);
+                }
+
                 do self.with_scope(Some(item.ident)) {
                     for foreign_item in foreign_module.items.iter() {
                         match foreign_item.node {
@@ -3681,6 +3714,7 @@ impl Resolver {
         }
 
         self.xray_context = orig_xray_flag;
+        self.path_all_public = orig_path_all_public;
     }
 
     pub fn with_type_parameter_rib(@mut self,
@@ -5464,8 +5498,9 @@ impl Resolver {
 
 pub struct CrateMap {
     def_map: DefMap,
+    trait_map: TraitMap,
     exp_map2: ExportMap2,
-    trait_map: TraitMap
+    exported_items: @mut ExportedItems,
 }
 
 /// Entry point to crate resolution.
@@ -5478,6 +5513,7 @@ pub fn resolve_crate(session: Session,
     CrateMap {
         def_map: resolver.def_map,
         exp_map2: resolver.export_map2,
+        exported_items: resolver.exported_items,
         trait_map: resolver.trait_map.clone(),
     }
 }