auto merge of #9735 : alexcrichton/rust/privacy, r=cmr

This is the culmination and attempted resolution of #8215. The commits have many more details about implementation details and the consequences of this refinement.

I'll point out specific locations which may be possible causes for alarm. In general, I have been very happy with how things have turned out. I'm a little sad that I couldn't remove privacy from resolve as much as I did, but I blame glob imports (although in theory even some of this can be mitigated as well).

Author: bors

doc/rust.md

@@ -1501,6 +1501,171 @@ is `extern "abi" fn(A1, ..., An) -> R`,
 where `A1...An` are the declared types of its arguments
 and `R` is the decalred return type.
 
+## Visibility and Privacy
+
+These two terms are often used interchangeably, and what they are attempting to
+convey is the answer to the question "Can this item be used at this location?"
+
+Rust's name resolution operates on a global hierarchy of namespaces. Each level
+in the hierarchy can be thought of as some item. The items are one of those
+mentioned above, but also include external crates. Declaring or defining a new
+module can be thought of as inserting a new tree into the hierarchy at the
+location of the definition.
+
+To control whether interfaces can be used across modules, Rust checks each use
+of an item to see whether it should be allowed or not. This is where privacy
+warnings are generated, or otherwise "you used a private item of another module
+and weren't allowed to."
+
+By default, everything in rust is *private*, with two exceptions. The first
+exception is that struct fields are public by default (but the struct itself is
+still private by default), and the remaining exception is that enum variants in
+a `pub` enum are the default visibility of the enum container itself.. You are
+allowed to alter this default visibility with the `pub` keyword (or `priv`
+keyword for struct fields and enum variants). When an item is declared as `pub`,
+it can be thought of as being accessible to the outside world. For example:
+
+~~~
+// Declare a private struct
+struct Foo;
+
+// Declare a public struct with a private field
+pub struct Bar {
+    priv field: int
+}
+
+// Declare a public enum with public and private variants
+pub enum State {
+    PubliclyAccessibleState,
+    priv PrivatelyAccessibleState
+}
+~~~
+
+With the notion of an item being either public or private, Rust allows item
+accesses in two cases:
+
+1. If an item is public, then it can be used externally through any of its
+   public ancestors.
+2. If an item is private, it may be accessed by the current module and its
+   descendants.
+
+These two cases are surprisingly powerful for creating module hierarchies
+exposing public APIs while hiding internal implementation details. To help
+explain, here's a few use cases and what they would entail.
+
+* A library developer needs to expose functionality to crates which link against
+  their library. As a consequence of the first case, this means that anything
+  which is usable externally must be `pub` from the root down to the destination
+  item. Any private item in the chain will disallow external accesses.
+
+* A crate needs a global available "helper module" to itself, but it doesn't
+  want to expose the helper module as a public API. To accomplish this, the root
+  of the crate's hierarchy would have a private module which then internally has
+  a "public api". Because the entire crate is an ancestor of the root, then the
+  entire local crate can access this private module through the second case.
+
+* When writing unit tests for a module, it's often a common idiom to have an
+  immediate child of the module to-be-tested named `mod test`. This module could
+  access any items of the parent module through the second case, meaning that
+  internal implementation details could also be seamlessly tested from the child
+  module.
+
+In the second case, it mentions that a private item "can be accessed" by the
+current module and its descendants, but the exact meaning of accessing an item
+depends on what the item is. Accessing a module, for example, would mean looking
+inside of it (to import more items). On the other hand, accessing a function
+would mean that it is invoked.
+
+Here's an example of a program which exemplifies the three cases outlined above.
+
+~~~
+// This module is private, meaning that no external crate can access this
+// module. Because it is private at the root of this current crate, however, any
+// module in the crate may access any publicly visible item in this module.
+mod crate_helper_module {
+
+    // This function can be used by anything in the current crate
+    pub fn crate_helper() {}
+
+    // This function *cannot* be used by anything else in the crate. It is not
+    // publicly visible outside of the `crate_helper_module`, so only this
+    // current module and its descendants may access it.
+    fn implementation_detail() {}
+}
+
+// This function is "public to the root" meaning that it's available to external
+// crates linking against this one.
+pub fn public_api() {}
+
+// Similarly to 'public_api', this module is public so external crates may look
+// inside of it.
+pub mod submodule {
+    use crate_helper_module;
+
+    pub fn my_method() {
+        // Any item in the local crate may invoke the helper module's public
+        // interface through a combination of the two rules above.
+        crate_helper_module::crate_helper();
+    }
+
+    // This function is hidden to any module which is not a descendant of
+    // `submodule`
+    fn my_implementation() {}
+
+    #[cfg(test)]
+    mod test {
+
+        #[test]
+        fn test_my_implementation() {
+            // Because this module is a descendant of `submodule`, it's allowed
+            // to access private items inside of `submodule` without a privacy
+            // violation.
+            super::my_implementation();
+        }
+    }
+}
+
+# fn main() {}
+~~~
+
+For a rust program to pass the privacy checking pass, all paths must be valid
+accesses given the two rules above. This includes all use statements,
+expressions, types, etc.
+
+### Re-exporting and Visibility
+
+Rust allows publicly re-exporting items through a `pub use` directive. Because
+this is a public directive, this allows the item to be used in the current
+module through the rules above. It essentially allows public access into the
+re-exported item. For example, this program is valid:
+
+~~~
+pub use api = self::implementation;
+
+mod implementation {
+    pub fn f() {}
+}
+
+# fn main() {}
+~~~
+
+This means that any external crate referencing `implementation::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
+chain" being short-circuited through the reexport instead of passing through the
+namespace hierarchy as it normally would.
+
+### Glob imports and Visibility
+
+Currently glob imports are considered an "experimental" language feature. For
+sanity purpose along with helping the implementation, glob imports will only
+import public items from their destination, not private items.
+
+> **Note:** This is subject to change, glob exports may be removed entirely or
+> they could possibly import private items for a privacy error to later be
+> issued if the item is used.
+
 ## Attributes
 
 ~~~~~~~~{.ebnf .gram}

doc/tutorial.md

@@ -2322,19 +2322,18 @@ fn main() {
 
 The `::farm::chicken` construct is what we call a 'path'.
 
-Because it's starting with a `::`, it's also a 'global path',
-which qualifies an item by its full path in the module hierarchy
-relative to the crate root.
+Because it's starting with a `::`, it's also a 'global path', which qualifies
+an item by its full path in the module hierarchy relative to the crate root.
 
-If the path were to start with a regular identifier, like `farm::chicken`, it would be
-a 'local path' instead. We'll get to them later.
+If the path were to start with a regular identifier, like `farm::chicken`, it
+would be a 'local path' instead. We'll get to them later.
 
-Now, if you actually tried to compile this code example, you'll notice
-that you get a `unresolved name: 'farm::chicken'` error. That's because per default,
-items (`fn`, `struct`, `static`, `mod`, ...) are only visible inside the module
-they are defined in.
+Now, if you actually tried to compile this code example, you'll notice that you
+get a `function 'chicken' is private` error. That's because by default, items
+(`fn`, `struct`, `static`, `mod`, ...) are private.
 
-To make them visible outside their containing modules, you need to mark them _public_ with `pub`:
+To make them visible outside their containing modules, you need to mark them
+_public_ with `pub`:
 
 ~~~~
 mod farm {
@@ -2356,7 +2355,8 @@ Rust doesn't support encapsulation: both struct fields and methods can
 be private. But this encapsulation is at the module level, not the
 struct level.
 
-For convenience, fields are _public_ by default, and can be made _private_ with the `priv` keyword:
+For convenience, fields are _public_ by default, and can be made _private_ with
+the `priv` keyword:
 
 ~~~
 mod farm {
@@ -2393,7 +2393,8 @@ fn main() {
 # fn make_me_a_chicken() -> farm::Chicken { 0 }
 ~~~
 
-> ***Note:*** Visibility rules are currently buggy and not fully defined, you might have to add or remove `pub` along a path until it works.
+Exact details and specifications about visibility rules can be found in the Rust
+manual.
 
 ## Files and modules
 

src/libextra/container.rs

@@ -40,7 +40,7 @@ pub trait Deque<T> : Mutable {
 }
 
 #[cfg(test)]
-mod bench {
+pub mod bench {
     use std::container::MutableMap;
     use std::{vec, rand};
     use std::rand::Rng;

src/libextra/crypto/cryptoutil.rs

@@ -346,7 +346,7 @@ impl <T: FixedBuffer> StandardPadding for T {
 
 
 #[cfg(test)]
-mod test {
+pub mod test {
     use std::rand::{IsaacRng, Rng};
     use std::vec;
 

src/libextra/stats.rs

@@ -101,7 +101,8 @@ pub trait Stats {
 
 /// Extracted collection of all the summary statistics of a sample set.
 #[deriving(Clone, Eq)]
-struct Summary {
+#[allow(missing_doc)]
+pub struct Summary {
     sum: f64,
     min: f64,
     max: f64,

src/librustc/driver/driver.rs

@@ -199,7 +199,6 @@ pub fn phase_2_configure_and_expand(sess: Session,
 
 pub struct CrateAnalysis {
     exp_map2: middle::resolve::ExportMap2,
-    exported_items: @middle::privacy::ExportedItems,
     ty_cx: ty::ctxt,
     maps: astencode::Maps,
     reachable: @mut HashSet<ast::NodeId>
@@ -229,7 +228,9 @@ 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,
+        external_exports: external_exports,
+        last_private_map: last_private_map
     } =
         time(time_passes, "resolution", (), |_|
              middle::resolve::resolve_crate(sess, lang_items, crate));
@@ -261,9 +262,10 @@ pub fn phase_3_run_analysis_passes(sess: Session,
          middle::check_const::check_crate(sess, crate, ast_map, def_map,
                                           method_map, ty_cx));
 
-    let exported_items =
-        time(time_passes, "privacy checking", (), |_|
-             middle::privacy::check_crate(ty_cx, &method_map, &exp_map2, crate));
+    let maps = (external_exports, last_private_map);
+    time(time_passes, "privacy checking", maps, |(a, b)|
+         middle::privacy::check_crate(ty_cx, &method_map, &exp_map2,
+                                      a, b, crate));
 
     time(time_passes, "effect checking", (), |_|
          middle::effect::check_crate(ty_cx, method_map, crate));
@@ -305,7 +307,6 @@ 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/decoder.rs

@@ -837,8 +837,9 @@ fn each_child_of_item_or_crate(intr: @ident_interner,
                 let def_like = item_to_def_like(child_item_doc,
                                                 child_def_id,
                                                 cdata.cnum);
-                callback(def_like, token::str_to_ident(name),
-                         item_visibility(child_item_doc));
+                // These items have a public visibility because they're part of
+                // a public re-export.
+                callback(def_like, token::str_to_ident(name), ast::public);
             }
         }
 

src/librustc/metadata/encoder.rs

@@ -58,7 +58,6 @@ pub struct EncodeParams<'self> {
     diag: @mut span_handler,
     tcx: ty::ctxt,
     reexports2: middle::resolve::ExportMap2,
-    exported_items: @middle::privacy::ExportedItems,
     item_symbols: &'self HashMap<ast::NodeId, ~str>,
     discrim_symbols: &'self HashMap<ast::NodeId, @str>,
     non_inlineable_statics: &'self HashSet<ast::NodeId>,
@@ -89,7 +88,6 @@ pub struct EncodeContext<'self> {
     tcx: ty::ctxt,
     stats: @mut Stats,
     reexports2: middle::resolve::ExportMap2,
-    exported_items: @middle::privacy::ExportedItems,
     item_symbols: &'self HashMap<ast::NodeId, ~str>,
     discrim_symbols: &'self HashMap<ast::NodeId, @str>,
     non_inlineable_statics: &'self HashSet<ast::NodeId>,
@@ -625,6 +623,7 @@ fn encode_info_for_mod(ecx: &EncodeContext,
     }
 
     encode_path(ecx, ebml_w, path, ast_map::path_mod(name));
+    encode_visibility(ebml_w, vis);
 
     // Encode the reexports of this module, if this module is public.
     if vis == public {
@@ -1277,12 +1276,7 @@ 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) };
-            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);
+            encode_info_for_item(ecx, &mut ebml_w, i, index, *pt, i.vis);
         }
         _ => fail2!("bad item")
     }
@@ -1628,7 +1622,7 @@ impl<'self> Visitor<()> for ImplVisitor<'self> {
 
                 // Load eagerly if this is an implementation of the Drop trait
                 // or if the trait is not defined in this crate.
-                if def_id == self.ecx.tcx.lang_items.drop_trait().unwrap() ||
+                if Some(def_id) == self.ecx.tcx.lang_items.drop_trait() ||
                         def_id.crate != LOCAL_CRATE {
                     self.ebml_w.start_tag(tag_impls_impl);
                     encode_def_id(self.ebml_w, local_def(item.id));
@@ -1744,7 +1738,6 @@ pub fn encode_metadata(parms: EncodeParams, crate: &Crate) -> ~[u8] {
         diag,
         tcx,
         reexports2,
-        exported_items,
         discrim_symbols,
         cstore,
         encode_inlined_item,
@@ -1760,7 +1753,6 @@ 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,
         non_inlineable_statics: non_inlineable_statics,

src/librustc/middle/borrowck/gather_loans/mod.rs

@@ -31,8 +31,8 @@ use syntax::ast_util::id_range;
 use syntax::codemap::Span;
 use syntax::print::pprust;
 use syntax::visit;
-use syntax::visit::Visitor;
-use syntax::ast::{Expr, fn_kind, fn_decl, Block, NodeId, Stmt, Pat, Local};
+use syntax::visit::{Visitor, fn_kind};
+use syntax::ast::{Expr, fn_decl, Block, NodeId, Stmt, Pat, Local};
 
 mod lifetime;
 mod restrictions;