Rollup merge of #23400 - nrc:pub_use, r=eddyb

r? @eddyb

src/doc/reference.md

@@ -1961,16 +1961,18 @@ module through the rules above. It essentially allows public access into the
 re-exported item. For example, this program is valid:
 
 ```
-pub use self::implementation as api;
+pub use self::implementation::api;
 
 mod implementation {
-    pub fn f() {}
+    pub mod api {
+        pub fn f() {}
+    }
 }
 
 # fn main() {}
 ```
 
-This means that any external crate referencing `implementation::f` would
+This means that any external crate referencing `implementation::api::f` would
 receive a privacy violation, while the path `api::f` would be allowed.
 
 When re-exporting a private item, it can be thought of as allowing the "privacy

src/librustc_resolve/build_reduced_graph.rs

@@ -14,16 +14,17 @@
 //! any imports resolved.
 
 use {DefModifiers, PUBLIC, IMPORTABLE};
-use ImportDirective;
-use ImportDirectiveSubclass::{self, SingleImport, GlobImport};
-use ImportResolution;
+use resolve_imports::ImportDirective;
+use resolve_imports::ImportDirectiveSubclass::{self, SingleImport, GlobImport};
+use resolve_imports::ImportResolution;
 use Module;
 use ModuleKind::*;
 use Namespace::{TypeNS, ValueNS};
 use NameBindings;
+use {names_to_string, module_to_string};
 use ParentLink::{self, ModuleParentLink, BlockParentLink};
 use Resolver;
-use Shadowable;
+use resolve_imports::Shadowable;
 use TypeNsDef;
 
 use self::DuplicateCheckingMode::*;
@@ -371,8 +372,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
             ItemExternCrate(_) => {
                 // n.b. we don't need to look at the path option here, because cstore already did
-                for &crate_id in self.session.cstore
-                                     .find_extern_mod_stmt_cnum(item.id).iter() {
+                if let Some(crate_id) = self.session.cstore.find_extern_mod_stmt_cnum(item.id) {
                     let def_id = DefId { krate: crate_id, node: 0 };
                     self.external_exports.insert(def_id);
                     let parent_link = ModuleParentLink(parent.downgrade(), name);
@@ -382,7 +382,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                                                               false,
                                                               true));
                     debug!("(build reduced graph for item) found extern `{}`",
-                            self.module_to_string(&*external_module));
+                            module_to_string(&*external_module));
                     self.check_for_conflicts_between_external_crates(&**parent, name, sp);
                     parent.external_module_children.borrow_mut()
                           .insert(name, external_module.clone());
@@ -400,7 +400,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                                             Some(def_id),
                                             NormalModuleKind,
                                             false,
-                                            item.vis == ast::Public,
+                                            is_public,
                                             sp);
 
                 name_bindings.get_module()
@@ -432,8 +432,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
             // These items live in the type namespace.
             ItemTy(..) => {
                 let name_bindings =
-                    self.add_child(name, parent, ForbidDuplicateTypesAndModules,
-                                   sp);
+                    self.add_child(name, parent, ForbidDuplicateTypesAndModules, sp);
 
                 name_bindings.define_type(DefTy(local_def(item.id), false), sp,
                                           modifiers);
@@ -517,7 +516,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                                             Some(local_def(item.id)),
                                             TraitModuleKind,
                                             false,
-                                            item.vis == ast::Public,
+                                            is_public,
                                             sp);
                 let module_parent = name_bindings.get_module();
 
@@ -636,8 +635,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                            name: Name,
                            new_parent: &Rc<Module>) {
         debug!("(building reduced graph for \
-                external crate) building external def, priv {:?}",
-               vis);
+                external crate) building external def {}, priv {:?}",
+               final_ident, vis);
         let is_public = vis == ast::Public;
         let modifiers = if is_public { PUBLIC } else { DefModifiers::empty() } | IMPORTABLE;
         let is_exported = is_public && match new_parent.def_id.get() {
@@ -667,7 +666,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
               Some(_) | None => {
                 debug!("(building reduced graph for \
                         external crate) building module \
-                        {}", final_ident);
+                        {} {}", final_ident, is_public);
                 let parent_link = self.get_parent_link(new_parent, name);
 
                 child_name_bindings.define_module(parent_link,
@@ -838,7 +837,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
     /// Builds the reduced graph rooted at the given external module.
     fn populate_external_module(&mut self, module: &Rc<Module>) {
         debug!("(populating external module) attempting to populate {}",
-               self.module_to_string(&**module));
+               module_to_string(&**module));
 
         let def_id = match module.def_id.get() {
             None => {
@@ -904,18 +903,14 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
         match subclass {
             SingleImport(target, _) => {
-                debug!("(building import directive) building import \
-                        directive: {}::{}",
-                       self.names_to_string(&module_.imports.borrow().last().unwrap().
-                                                             module_path),
+                debug!("(building import directive) building import directive: {}::{}",
+                       names_to_string(&module_.imports.borrow().last().unwrap().module_path),
                        token::get_name(target));
 
-                let mut import_resolutions = module_.import_resolutions
-                                                    .borrow_mut();
+                let mut import_resolutions = module_.import_resolutions.borrow_mut();
                 match import_resolutions.get_mut(&target) {
                     Some(resolution) => {
-                        debug!("(building import directive) bumping \
-                                reference");
+                        debug!("(building import directive) bumping reference");
                         resolution.outstanding_references += 1;
 
                         // the source of this name is different now

src/librustc_resolve/diagnostics.rs

@@ -24,7 +24,9 @@ register_diagnostics! {
     E0258, // import conflicts with existing submodule
     E0259, // an extern crate has already been imported into this module
     E0260, // name conflicts with an external crate that has been imported into this module
-    E0317 // user-defined types or type parameters cannot shadow the primitive types
+    E0317, // user-defined types or type parameters cannot shadow the primitive types
+    E0364, // item is private
+    E0365  // item is private
 }
 
 __build_diagnostic_array! { DIAGNOSTICS }

src/librustc_resolve/record_exports.rs

@@ -22,6 +22,7 @@ use {Module, NameBindings, Resolver};
 use Namespace::{self, TypeNS, ValueNS};
 
 use build_reduced_graph;
+use module_to_string;
 
 use rustc::middle::def::Export;
 use syntax::ast;
@@ -60,19 +61,19 @@ impl<'a, 'b, 'tcx> ExportRecorder<'a, 'b, 'tcx> {
                 // OK. Continue.
                 debug!("(recording exports for module subtree) recording \
                         exports for local module `{}`",
-                       self.module_to_string(&*module_));
+                       module_to_string(&*module_));
             }
             None => {
                 // Record exports for the root module.
                 debug!("(recording exports for module subtree) recording \
                         exports for root module `{}`",
-                       self.module_to_string(&*module_));
+                       module_to_string(&*module_));
             }
             Some(_) => {
                 // Bail out.
                 debug!("(recording exports for module subtree) not recording \
                         exports for `{}`",
-                       self.module_to_string(&*module_));
+                       module_to_string(&*module_));
                 return;
             }
         }
@@ -133,13 +134,13 @@ impl<'a, 'b, 'tcx> ExportRecorder<'a, 'b, 'tcx> {
     fn add_exports_for_module(&mut self,
                               exports: &mut Vec<Export>,
                               module_: &Module) {
-        for (name, importresolution) in &*module_.import_resolutions.borrow() {
-            if !importresolution.is_public {
+        for (name, import_resolution) in &*module_.import_resolutions.borrow() {
+            if !import_resolution.is_public {
                 continue
             }
             let xs = [TypeNS, ValueNS];
             for &ns in &xs {
-                match importresolution.target_for_namespace(ns) {
+                match import_resolution.target_for_namespace(ns) {
                     Some(target) => {
                         debug!("(computing exports) maybe export '{}'",
                                token::get_name(*name));

src/libsyntax/test.rs

@@ -121,13 +121,11 @@ impl<'a> fold::Folder for TestHarnessGenerator<'a> {
         debug!("current path: {}",
                ast_util::path_name_i(&self.cx.path));
 
-        if is_test_fn(&self.cx, &*i) || is_bench_fn(&self.cx, &*i) {
+        let i = if is_test_fn(&self.cx, &*i) || is_bench_fn(&self.cx, &*i) {
             match i.node {
                 ast::ItemFn(_, ast::Unsafety::Unsafe, _, _, _) => {
                     let diag = self.cx.span_diagnostic;
-                    diag.span_fatal(i.span,
-                                    "unsafe functions cannot be used for \
-                                     tests");
+                    diag.span_fatal(i.span, "unsafe functions cannot be used for tests");
                 }
                 _ => {
                     debug!("this is a test function");
@@ -142,9 +140,19 @@ impl<'a> fold::Folder for TestHarnessGenerator<'a> {
                     self.tests.push(i.ident);
                     // debug!("have {} test/bench functions",
                     //        cx.testfns.len());
+
+                    // Make all tests public so we can call them from outside
+                    // the module (note that the tests are re-exported and must
+                    // be made public themselves to avoid privacy errors).
+                    i.map(|mut i| {
+                        i.vis = ast::Public;
+                        i
+                    })
                 }
             }
-        }
+        } else {
+            i
+        };
 
         // We don't want to recurse into anything other than mods, since
         // mods or tests inside of functions will break things

src/test/auxiliary/privacy_reexport.rs

@@ -10,6 +10,6 @@
 
 pub use foo as bar;
 
-mod foo {
+pub mod foo {
     pub fn frob() {}
 }

src/test/compile-fail/lint-dead-code-3.rs

@@ -19,7 +19,7 @@ extern crate libc;
 
 pub use extern_foo as x;
 extern {
-    fn extern_foo();
+    pub fn extern_foo();
 }
 
 struct Foo; //~ ERROR: struct is never used

src/test/compile-fail/privacy1.rs

@@ -27,10 +27,6 @@ mod bar {
 
     // can't publicly re-export private items
     pub use self::baz::{foo, bar};
-    //~^ ERROR: function `bar` is private
-
-    pub use self::private::ppriv;
-    //~^ ERROR: function `ppriv` is private
 
     pub struct A;
     impl A {
@@ -61,10 +57,8 @@ mod bar {
             fn bar2(&self) {}
         }
 
-        // both of these are re-exported by `bar`, but only one should be
-        // validly re-exported
         pub fn foo() {}
-        fn bar() {}
+        pub fn bar() {}
     }
 
     extern {
@@ -92,10 +86,6 @@ mod bar {
         pub fn gpub() {}
         fn gpriv() {}
     }
-
-    mod private {
-        fn ppriv() {}
-    }
 }
 
 pub fn gpub() {}
@@ -142,13 +132,13 @@ mod foo {
 
         ::bar::baz::foo(); //~ ERROR: function `foo` is inaccessible
                            //~^ NOTE: module `baz` is private
-        ::bar::baz::bar(); //~ ERROR: function `bar` is private
+        ::bar::baz::bar(); //~ ERROR: function `bar` is inaccessible
     }
 
     fn test2() {
         use bar::baz::{foo, bar};
         //~^ ERROR: function `foo` is inaccessible
-        //~^^ ERROR: function `bar` is private
+        //~^^ ERROR: function `bar` is inaccessible
         foo();
         bar();
     }

src/test/run-pass/issue-16597.rs

@@ -15,5 +15,5 @@ mod test {
     use super::*;
 
     #[test]
-    fn test(){}
+    pub fn test(){}
 }

src/test/run-pass/issue-20823.rs

@@ -14,4 +14,4 @@
 #![deny(unstable)]
 
 #[test]
-fn foo() {}
+pub fn foo() {}

src/test/run-pass/issue-5950.rs

@@ -11,6 +11,6 @@
 
 pub use local as local_alias;
 
-mod local { }
+pub mod local { }
 
 pub fn main() {}

src/test/run-pass/test-fn-signature-verification-for-explicit-return-type.rs

@@ -13,8 +13,8 @@
 extern crate test;
 
 #[bench]
-fn bench_explicit_return_type(_: &mut ::test::Bencher) -> () {}
+pub fn bench_explicit_return_type(_: &mut ::test::Bencher) -> () {}
 
 #[test]
-fn test_explicit_return_type() -> () {}
+pub fn test_explicit_return_type() -> () {}
 

src/test/run-pass/test-should-fail-good-message.rs

@@ -13,13 +13,13 @@
 
 #[test]
 #[should_panic(expected = "foo")]
-fn test_foo() {
+pub fn test_foo() {
     panic!("foo bar")
 }
 
 #[test]
 #[should_panic(expected = "foo")]
-fn test_foo_dynamic() {
+pub fn test_foo_dynamic() {
     panic!("{} bar", "foo")
 }