librustc: Fix privacy checking for cross-crate variants

7 files changed, 81 insertions, 34 deletions

diff --git a/src/librustc/metadata/csearch.rs b/src/librustc/metadata/csearch.rs
index 9e002183137..b21417f5100 100644
--- a/src/librustc/metadata/csearch.rs
+++ b/src/librustc/metadata/csearch.rs
@@ -52,9 +52,22 @@ pub fn each_lang_item(cstore: @mut cstore::CStore,
 }
 
 /// Iterates over all the paths in the given crate.
+#[cfg(stage0)]
 pub fn each_path(cstore: @mut cstore::CStore,
                  cnum: ast::crate_num,
-                 f: &fn(&str, decoder::def_like) -> bool) -> bool {
+                 f: &fn(&str, decoder::def_like, ast::visibility) -> bool) {
+    let crate_data = cstore::get_crate_data(cstore, cnum);
+    let get_crate_data: decoder::GetCrateDataCb = |cnum| {
+        cstore::get_crate_data(cstore, cnum)
+    };
+    decoder::each_path(cstore.intr, crate_data, get_crate_data, f)
+}
+/// Iterates over all the paths in the given crate.
+#[cfg(not(stage0))]
+pub fn each_path(cstore: @mut cstore::CStore,
+                 cnum: ast::crate_num,
+                 f: &fn(&str, decoder::def_like, ast::visibility) -> bool)
+                 -> bool {
     let crate_data = cstore::get_crate_data(cstore, cnum);
     let get_crate_data: decoder::GetCrateDataCb = |cnum| {
         cstore::get_crate_data(cstore, cnum)
diff --git a/src/librustc/metadata/decoder.rs b/src/librustc/metadata/decoder.rs
index de440632b1f..d6e7e64e4f8 100644
--- a/src/librustc/metadata/decoder.rs
+++ b/src/librustc/metadata/decoder.rs
@@ -319,8 +319,7 @@ fn item_name(intr: @ident_interner, item: ebml::Doc) -> ast::ident {
 }
 
 fn item_to_def_like(item: ebml::Doc, did: ast::def_id, cnum: ast::crate_num)
-    -> def_like
-{
+    -> def_like {
     let fam = item_family(item);
     match fam {
         Const     => dl_def(ast::def_const(did)),
@@ -474,9 +473,11 @@ pub fn each_lang_item(cdata: cmd, f: &fn(ast::node_id, uint) -> bool) -> bool {
 }
 
 /// Iterates over all the paths in the given crate.
-pub fn _each_path(intr: @ident_interner, cdata: cmd,
+pub fn _each_path(intr: @ident_interner,
+                  cdata: cmd,
                   get_crate_data: GetCrateDataCb,
-                  f: &fn(&str, def_like) -> bool) -> bool {
+                  f: &fn(&str, def_like, ast::visibility) -> bool)
+                  -> bool {
     let root = reader::Doc(cdata.data);
     let items = reader::get_doc(root, tag_items);
     let items_data = reader::get_doc(items, tag_items_data);
@@ -497,8 +498,10 @@ pub fn _each_path(intr: @ident_interner, cdata: cmd,
                 debug!("(each_path) yielding explicit item: %s", path);
                 let def_like = item_to_def_like(item_doc, def_id, cdata.cnum);
 
+                let vis = item_visibility(item_doc);
+
                 // Hand the information off to the iteratee.
-                if !f(path, def_like) {
+                if !f(path, def_like, vis) {
                     broken = true;      // FIXME #4572: This is awful.
                 }
             }
@@ -548,7 +551,7 @@ pub fn _each_path(intr: @ident_interner, cdata: cmd,
                             debug!("(each_path) yielding reexported \
                                     item: %s", reexport_path);
 
-                            if (!f(reexport_path, def_like)) {
+                            if (!f(reexport_path, def_like, ast::public)) {
                                 broken = true;  // FIXME #4572: This is awful.
                             }
                         }
@@ -561,9 +564,19 @@ pub fn _each_path(intr: @ident_interner, cdata: cmd,
     return broken;
 }
 
-pub fn each_path(intr: @ident_interner, cdata: cmd,
+#[cfg(stage0)]
+pub fn each_path(intr: @ident_interner,
+                 cdata: cmd,
+                 get_crate_data: GetCrateDataCb,
+                 f: &fn(&str, def_like, ast::visibility) -> bool) {
+    _each_path(intr, cdata, get_crate_data, f);
+}
+#[cfg(not(stage0))]
+pub fn each_path(intr: @ident_interner,
+                 cdata: cmd,
                  get_crate_data: GetCrateDataCb,
-                 f: &fn(&str, def_like) -> bool) -> bool {
+                 f: &fn(&str, def_like, ast::visibility) -> bool)
+                 -> bool {
     _each_path(intr, cdata, get_crate_data, f)
 }
 
@@ -1127,7 +1140,7 @@ pub fn get_crate_vers(data: @~[u8]) -> @~str {
 fn iter_crate_items(intr: @ident_interner, cdata: cmd,
                     get_crate_data: GetCrateDataCb,
                     proc: &fn(path: &str, ast::def_id)) {
-    for each_path(intr, cdata, get_crate_data) |path_string, def_like| {
+    for each_path(intr, cdata, get_crate_data) |path_string, def_like, _| {
         match def_like {
             dl_impl(*) | dl_field => {}
             dl_def(def) => {
diff --git a/src/librustc/metadata/encoder.rs b/src/librustc/metadata/encoder.rs
index 5fc05fe7920..297b0408dc5 100644
--- a/src/librustc/metadata/encoder.rs
+++ b/src/librustc/metadata/encoder.rs
@@ -316,6 +316,7 @@ fn encode_enum_variant_info(ecx: @EncodeContext,
         encode_family(ebml_w, 'v');
         encode_name(ecx, ebml_w, variant.node.name);
         encode_parent_item(ebml_w, local_def(id));
+        encode_visibility(ebml_w, variant.node.vis);
         encode_type(ecx, ebml_w,
                     node_id_to_type(ecx.tcx, variant.node.id));
         match variant.node.kind {
diff --git a/src/librustc/middle/resolve.rs b/src/librustc/middle/resolve.rs
index bfa4862e054..306671641a3 100644
--- a/src/librustc/middle/resolve.rs
+++ b/src/librustc/middle/resolve.rs
@@ -539,7 +539,8 @@ pub impl NameBindings {
                 self.type_def = Some(TypeNsDef {
                     privacy: privacy,
                     module_def: Some(module),
-                    type_def: None
+                    type_def: None,
+                    type_span: None,
                 })
             }
             Some(type_def) => {
@@ -549,7 +550,8 @@ pub impl NameBindings {
                         self.type_def = Some(TypeNsDef {
                             privacy: privacy,
                             module_def: Some(module),
-                            type_def: type_def.type_def
+                            type_def: type_def.type_def,
+                            type_span: None,
                         })
                     }
                     Some(module_def) => module_def.kind = kind,
@@ -1392,10 +1394,8 @@ pub impl Resolver {
         }
     }
 
-    /**
-     * Constructs the reduced graph for one 'view item'. View items consist
-     * of imports and use directives.
-     */
+    /// Constructs the reduced graph for one 'view item'. View items consist
+    /// of imports and use directives.
     fn build_reduced_graph_for_view_item(@mut self,
                                          view_item: @view_item,
                                          parent: ReducedGraphParent,
@@ -1551,11 +1551,13 @@ pub impl Resolver {
 
     fn handle_external_def(@mut self,
                            def: def,
+                           visibility: ast::visibility,
                            modules: &mut HashMap<def_id, @mut Module>,
                            child_name_bindings: @mut NameBindings,
                            final_ident: &str,
                            ident: ident,
                            new_parent: ReducedGraphParent) {
+        let privacy = visibility_to_privacy(visibility);
         match def {
           def_mod(def_id) | def_foreign_mod(def_id) => {
             match child_name_bindings.type_def {
@@ -1573,7 +1575,7 @@ pub impl Resolver {
 
                 // FIXME (#5074): this should be a match on find
                 if !modules.contains_key(&def_id) {
-                    child_name_bindings.define_module(Public,
+                    child_name_bindings.define_module(privacy,
                                                       parent_link,
                                                       Some(def_id),
                                                       NormalModuleKind,
@@ -1582,9 +1584,9 @@ pub impl Resolver {
                                    child_name_bindings.get_module());
                 } else {
                     let existing_module = *modules.get(&def_id);
-                    // Create an import resolution to
-                    // avoid creating cycles in the
-                    // module graph.
+
+                    // Create an import resolution to avoid creating cycles in
+                    // the module graph.
 
                     let resolution = @mut ImportResolution(Public, 0);
                     resolution.outstanding_references = 0;
@@ -1610,11 +1612,19 @@ pub impl Resolver {
               }
             }
           }
-          def_fn(*) | def_static_method(*) | def_const(*) |
           def_variant(*) => {
+            debug!("(building reduced graph for external crate) building \
+                    variant %s",
+                   final_ident);
+            // We assume the parent is visible, or else we wouldn't have seen
+            // it.
+            let privacy = variant_visibility_to_privacy(visibility, true);
+            child_name_bindings.define_value(privacy, def, dummy_sp());
+          }
+          def_fn(*) | def_static_method(*) | def_const(*) => {
             debug!("(building reduced graph for external \
                     crate) building value %s", final_ident);
-            child_name_bindings.define_value(Public, def, dummy_sp());
+            child_name_bindings.define_value(privacy, def, dummy_sp());
           }
           def_trait(def_id) => {
               debug!("(building reduced graph for external \
@@ -1651,11 +1661,11 @@ pub impl Resolver {
                   }
               }
 
-              child_name_bindings.define_type(Public, def, dummy_sp());
+              child_name_bindings.define_type(privacy, def, dummy_sp());
 
               // Define a module if necessary.
               let parent_link = self.get_parent_link(new_parent, ident);
-              child_name_bindings.set_module_kind(Public,
+              child_name_bindings.set_module_kind(privacy,
                                                   parent_link,
                                                   Some(def_id),
                                                   TraitModuleKind,
@@ -1665,13 +1675,13 @@ pub impl Resolver {
               debug!("(building reduced graph for external \
                       crate) building type %s", final_ident);
 
-              child_name_bindings.define_type(Public, def, dummy_sp());
+              child_name_bindings.define_type(privacy, def, dummy_sp());
           }
           def_struct(def_id) => {
             debug!("(building reduced graph for external \
                     crate) building type %s",
                    final_ident);
-            child_name_bindings.define_type(Public, def, dummy_sp());
+            child_name_bindings.define_type(privacy, def, dummy_sp());
             self.structs.insert(def_id);
           }
           def_self(*) | def_arg(*) | def_local(*) |
@@ -1692,7 +1702,7 @@ pub impl Resolver {
 
         // Create all the items reachable by paths.
         for each_path(self.session.cstore, root.def_id.get().crate)
-                |path_string, def_like| {
+                |path_string, def_like, visibility| {
 
             debug!("(building reduced graph for external crate) found path \
                         entry: %s (%?)",
@@ -1760,6 +1770,7 @@ pub impl Resolver {
                                        dummy_sp());
 
                     self.handle_external_def(def,
+                                             visibility,
                                              &mut modules,
                                              child_name_bindings,
                                              *self.session.str_of(
@@ -2995,14 +3006,9 @@ pub impl Resolver {
 
         // If this is a search of all imports, we should be done with glob
         // resolution at this point.
-<<<<<<< HEAD
-        if name_search_type == SearchItemsAndAllImports {
-            assert_eq!(module_.glob_count, 0);
-=======
         if name_search_type == PathPublicOrPrivateSearch ||
                 name_search_type == PathPublicOnlySearch {
-            assert!(module_.glob_count == 0);
->>>>>>> librustc: Disallow `use` from reaching into impls or traits.
+            assert_eq!(module_.glob_count, 0);
         }
 
         // Check the list of resolved imports.
diff --git a/src/librustc/middle/typeck/coherence.rs b/src/librustc/middle/typeck/coherence.rs
index f133a010f83..8870f0fdc57 100644
--- a/src/librustc/middle/typeck/coherence.rs
+++ b/src/librustc/middle/typeck/coherence.rs
@@ -975,7 +975,7 @@ pub impl CoherenceChecker {
                                       def_id { crate: crate_number,
                                                node: 0 });
 
-            for each_path(crate_store, crate_number) |_p, def_like| {
+            for each_path(crate_store, crate_number) |_, def_like, _| {
                 match def_like {
                     dl_def(def_mod(def_id)) => {
                         self.add_impls_for_module(&mut impls_seen,
diff --git a/src/test/auxiliary/private_variant_xc.rs b/src/test/auxiliary/private_variant_xc.rs
new file mode 100644
index 00000000000..d7d55c691b6
--- /dev/null
+++ b/src/test/auxiliary/private_variant_xc.rs
@@ -0,0 +1,5 @@
+pub enum Foo {
+    pub Bar,
+    priv Baz,
+}
+
diff --git a/src/test/compile-fail/private-variant-xc.rs b/src/test/compile-fail/private-variant-xc.rs
new file mode 100644
index 00000000000..c7838b98855
--- /dev/null
+++ b/src/test/compile-fail/private-variant-xc.rs
@@ -0,0 +1,9 @@
+// aux-build:private_variant_xc.rs
+
+extern mod private_variant_xc;
+
+pub fn main() {
+    let _ = private_variant_xc::Bar;
+    let _ = private_variant_xc::Baz;    //~ ERROR unresolved name
+}
+