From 439e2770be6aec41a3961235305787a78d47fbdd Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 5 Oct 2013 14:37:39 -0700 Subject: [PATCH 1/4] Extract privacy checking from name resolution This commit is the culmination of my recent effort to refine Rust's notion of privacy and visibility among crates. The major goals of this commit were to remove privacy checking from resolve for the sake of sane error messages, and to attempt a much more rigid and well-tested implementation of visibility throughout rust. The implemented rules for name visibility are: 1. Everything pub from the root namespace is visible to anyone 2. You may access any private item of your ancestors. "Accessing a private item" depends on what the item is, so for a function this means that you can call it, but for a module it means that you can look inside of it. Once you look inside a private module, any accessed item must be "pub from the root" where the new root is the private module that you looked into. These rules required some more analysis results to get propagated from trans to privacy in the form of a few hash tables. I added a new test in which my goal was to showcase all of the privacy nuances of the language, and I hope to place any new bugs into this file to prevent regressions. Overall, I was unable to completely remove the notion of privacy from resolve. One use of privacy is for dealing with glob imports. Essentially a glob import can only import *public* items from the destination, and because this must be done at namespace resolution time, resolve must maintain the notion of "what items are public in a module". There are some sad approximations of privacy, but I unfortunately can't see clear methods to extract them outside. The other use case of privacy in resolve now is one that must stick around regardless of glob imports. When dealing with privacy, checking a private path needs to know "what the last private thing was" when looking at a path. Resolve is the only compiler pass which knows the answer to this question, so it maintains the answer on a per-path resolution basis (works similarly to the def_map generated). Closes #8215 --- src/librustc/driver/driver.rs | 13 +- src/librustc/metadata/decoder.rs | 5 +- src/librustc/metadata/encoder.rs | 13 +- src/librustc/middle/privacy.rs | 1222 +++++++++++++----------- src/librustc/middle/resolve.rs | 965 ++++++++++--------- src/librustc/middle/trans/base.rs | 2 - src/librustc/middle/trans/context.rs | 3 - src/libsyntax/ast_util.rs | 26 - src/test/compile-fail/glob-resolve1.rs | 45 + src/test/compile-fail/privacy1.rs | 168 ++++ src/test/compile-fail/privacy2.rs | 37 + src/test/compile-fail/privacy3.rs | 32 + src/test/compile-fail/privacy4.rs | 31 + 13 files changed, 1459 insertions(+), 1103 deletions(-) create mode 100644 src/test/compile-fail/glob-resolve1.rs create mode 100644 src/test/compile-fail/privacy1.rs create mode 100644 src/test/compile-fail/privacy2.rs create mode 100644 src/test/compile-fail/privacy3.rs create mode 100644 src/test/compile-fail/privacy4.rs diff --git a/src/librustc/driver/driver.rs b/src/librustc/driver/driver.rs index 5534342f3c7fe..4681c37fce833 100644 --- a/src/librustc/driver/driver.rs +++ b/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 @@ -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, diff --git a/src/librustc/metadata/decoder.rs b/src/librustc/metadata/decoder.rs index 5c6a7c4f3b7c7..be01d3cbfc9d5 100644 --- a/src/librustc/metadata/decoder.rs +++ b/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); } } diff --git a/src/librustc/metadata/encoder.rs b/src/librustc/metadata/encoder.rs index e88ee70105195..ad190dfcfb127 100644 --- a/src/librustc/metadata/encoder.rs +++ b/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, discrim_symbols: &'self HashMap, non_inlineable_statics: &'self HashSet, @@ -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, discrim_symbols: &'self HashMap, non_inlineable_statics: &'self HashSet, @@ -1277,12 +1275,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 +1621,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 +1737,6 @@ pub fn encode_metadata(parms: EncodeParams, crate: &Crate) -> ~[u8] { diag, tcx, reexports2, - exported_items, discrim_symbols, cstore, encode_inlined_item, @@ -1760,7 +1752,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, diff --git a/src/librustc/middle/privacy.rs b/src/librustc/middle/privacy.rs index 7088091e1928c..f395231124b64 100644 --- a/src/librustc/middle/privacy.rs +++ b/src/librustc/middle/privacy.rs @@ -12,221 +12,362 @@ //! outside their scopes. This pass will also generate a set of exported items //! which are available for use externally when compiled as a library. -use std::hashmap::HashSet; +use std::hashmap::{HashSet, HashMap}; -use metadata::csearch; -use middle::resolve::ExportMap2; -use middle::ty::{ty_struct, ty_enum}; +use middle::resolve; use middle::ty; use middle::typeck::{method_map, method_origin, method_param}; use middle::typeck::{method_static, method_object}; -use std::util::ignore; -use syntax::ast::{DeclItem, Def, DefFn, DefId, DefStaticMethod}; -use syntax::ast::{DefVariant, ExprField, ExprMethodCall, ExprPath}; -use syntax::ast::{ExprStruct, ExprUnary, Ident, inherited, item_enum}; -use syntax::ast::{item_foreign_mod, item_fn, item_impl, item_struct}; -use syntax::ast::{item_trait, LOCAL_CRATE, NodeId, PatStruct, Path}; -use syntax::ast::{private, provided, public, required, StmtDecl, visibility}; use syntax::ast; -use syntax::ast_map::{node_foreign_item, node_item, node_method}; -use syntax::ast_map::{node_trait_method}; use syntax::ast_map; -use syntax::ast_util::{Private, Public, is_local}; -use syntax::ast_util::{variant_visibility_to_privacy, visibility_to_privacy}; +use syntax::ast_util::is_local; use syntax::attr; use syntax::codemap::Span; use syntax::parse::token; +use syntax::opt_vec; use syntax::visit; use syntax::visit::Visitor; -use syntax::ast::{_mod,Expr,item,Block,Pat}; -// 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; +type Context<'self> = (&'self method_map, &'self resolve::ExportMap2); -type Context<'self> = (&'self method_map, &'self ExportMap2); - -struct PrivacyVisitor { - tcx: ty::ctxt, - privileged_items: @mut ~[NodeId], - - // A set of all items which are re-exported to be used across crates - exported_items: ExportedItems, - - // A flag as to whether the current path is public all the way down to the - // current point or not - path_all_public: bool, +// This visitor is used to determine the parent of all nodes in question when it +// comes to privacy. This is used to determine later on if a usage is actually +// valid or not. +struct ParentVisitor<'self> { + parents: &'self mut HashMap, + curparent: ast::NodeId, } -impl PrivacyVisitor { - // Adds an item to its scope. - fn add_privileged_item(&mut self, item: @ast::item, count: &mut uint) { +impl<'self> Visitor<()> for ParentVisitor<'self> { + fn visit_item(&mut self, item: @ast::item, _: ()) { + self.parents.insert(item.id, self.curparent); + + let prev = self.curparent; match item.node { - item_struct(*) | item_trait(*) | item_enum(*) | - item_fn(*) => { - self.privileged_items.push(item.id); - *count += 1; - } - item_impl(_, _, _, ref methods) => { - for method in methods.iter() { - self.privileged_items.push(method.id); - *count += 1; - } - self.privileged_items.push(item.id); - *count += 1; - } - item_foreign_mod(ref foreign_mod) => { - for foreign_item in foreign_mod.items.iter() { - self.privileged_items.push(foreign_item.id); - *count += 1; + ast::item_mod(*) => { self.curparent = item.id; } + // Enum variants are parented to the enum definition itself beacuse + // they inherit privacy + ast::item_enum(ref def, _) => { + for variant in def.variants.iter() { + // If variants are private, then their logical "parent" is + // the enclosing module because everyone in the enclosing + // module can still use the private variant + if variant.node.vis == ast::private { + self.parents.insert(variant.node.id, self.curparent); + + // Otherwise, if the variant is public, then the parent is + // considered the enclosing enum because the enum will + // dictate the privacy visibility of this variant instead. + } else { + self.parents.insert(variant.node.id, item.id); + } } } _ => {} } + visit::walk_item(self, item, ()); + self.curparent = prev; } - // Adds items that are privileged to this scope. - fn add_privileged_items(&mut self, items: &[@ast::item]) -> uint { - let mut count = 0; - for &item in items.iter() { - self.add_privileged_item(item, &mut count); - } - count + fn visit_trait_method(&mut self, m: &ast::trait_method, _: ()) { + match *m { + ast::provided(ref m) => self.parents.insert(m.id, self.curparent), + ast::required(ref m) => self.parents.insert(m.id, self.curparent), + }; + visit::walk_trait_method(self, m, ()); } - // Checks that an enum variant is in scope - fn check_variant(&mut self, span: Span, enum_id: ast::DefId) { - let variant_info = ty::enum_variants(self.tcx, enum_id)[0]; - let parental_privacy = if is_local(enum_id) { - let parent_vis = ast_map::node_item_query(self.tcx.items, - enum_id.node, - |it| { it.vis }, - ~"unbound enum parent when checking \ - dereference of enum type"); - visibility_to_privacy(parent_vis) + fn visit_foreign_item(&mut self, a: @ast::foreign_item, _: ()) { + self.parents.insert(a.id, self.curparent); + visit::walk_foreign_item(self, a, ()); + } + + fn visit_fn(&mut self, a: &visit::fn_kind, b: &ast::fn_decl, + c: &ast::Block, d: Span, id: ast::NodeId, _: ()) { + self.parents.insert(id, self.curparent); + visit::walk_fn(self, a, b, c, d, id, ()); + } + + fn visit_struct_def(&mut self, s: @ast::struct_def, i: ast::Ident, + g: &ast::Generics, n: ast::NodeId, _: ()) { + // Struct constructors are parented to their struct definitions because + // they essentially are the struct definitions. + match s.ctor_id { + Some(id) => { self.parents.insert(id, n); } + None => {} } - else { - // WRONG - Public - }; - debug2!("parental_privacy = {:?}", parental_privacy); - debug2!("vis = {:?}, priv = {:?}", - variant_info.vis, - visibility_to_privacy(variant_info.vis)) - // inherited => privacy of the enum item - if variant_visibility_to_privacy(variant_info.vis, - parental_privacy == Public) - == Private { - self.tcx.sess.span_err(span, - "can only dereference enums \ - with a single, public variant"); + + // While we have the id of the struct definition, go ahead and parent + // all the fields. + for field in s.fields.iter() { + let vis = match field.node.kind { + ast::named_field(_, vis) => vis, + ast::unnamed_field => continue + }; + + // Private fields are scoped to this module, so parent them directly + // to the module instead of the struct. This is similar to the case + // of private enum variants. + if vis == ast::private { + self.parents.insert(field.node.id, self.curparent); + + // Otherwise public fields are scoped to the visibility of the + // struct itself + } else { + self.parents.insert(field.node.id, n); + } } + visit::walk_struct_def(self, s, i, g, n, ()) } +} - // Returns true if a crate-local method is private and false otherwise. - fn method_is_private(&mut self, span: Span, method_id: NodeId) -> bool { - let check = |vis: visibility, container_id: DefId| { - let mut is_private = false; - if vis == private { - is_private = true; - } else if vis == public { - is_private = false; - } else { - // Look up the enclosing impl. - if container_id.crate != LOCAL_CRATE { - self.tcx.sess.span_bug(span, - "local method isn't in local \ - impl?!"); - } +// This visitor is used to determine which items of the ast are embargoed, +// otherwise known as not exported. +struct EmbargoVisitor<'self> { + exported_items: &'self mut HashSet, + exp_map2: &'self resolve::ExportMap2, + path_all_public: bool, +} - match self.tcx.items.find(&container_id.node) { - Some(&node_item(item, _)) => { - match item.node { - item_impl(_, None, _, _) - if item.vis != public => { - is_private = true; - } - _ => {} - } - } - Some(_) => { - self.tcx.sess.span_bug(span, "impl wasn't an item?!"); - } - None => { - self.tcx.sess.span_bug(span, "impl wasn't in AST map?!"); +impl<'self> Visitor<()> for EmbargoVisitor<'self> { + fn visit_item(&mut self, item: @ast::item, _: ()) { + let orig_all_pub = self.path_all_public; + match item.node { + // impls/extern blocks do not break the "public chain" because they + // cannot have visibility qualifiers on them anyway + ast::item_impl(*) | ast::item_foreign_mod(*) => {} + + // Private by default, hence we only retain the "public chain" if + // `pub` is explicitly listed. + _ => { + self.path_all_public = orig_all_pub && item.vis == ast::public; + } + } + + if self.path_all_public { + self.exported_items.insert(item.id); + } + + match item.node { + // Enum variants inherit from their parent, so if the enum is + // public all variants are public unless they're explicitly priv + ast::item_enum(ref def, _) if self.path_all_public => { + for variant in def.variants.iter() { + if variant.node.vis != ast::private { + self.exported_items.insert(variant.node.id); } } } - is_private - }; - - match self.tcx.items.find(&method_id) { - Some(&node_method(method, impl_id, _)) => { - check(method.vis, impl_id) + // Methods which are public at the source are totally public. + ast::item_impl(_, None, _, ref methods) => { + for method in methods.iter() { + let public = match method.explicit_self.node { + ast::sty_static => self.path_all_public, + _ => true, + } && method.vis == ast::public; + if public { + self.exported_items.insert(method.id); + } + } } - Some(&node_trait_method(trait_method, trait_id, _)) => { - match *trait_method { - required(_) => check(public, trait_id), - provided(method) => check(method.vis, trait_id), + + // Trait implementation methods are all completely public + ast::item_impl(_, Some(*), _, ref methods) => { + for method in methods.iter() { + debug2!("exporting: {}", method.id); + self.exported_items.insert(method.id); } } - Some(_) => { - self.tcx.sess.span_bug(span, - format!("method_is_private: method was a {}?!", - ast_map::node_id_to_str( - self.tcx.items, - method_id, - token::get_ident_interner()))); + + // Default methods on traits are all public so long as the trait is + // public + ast::item_trait(_, _, ref methods) if self.path_all_public => { + for method in methods.iter() { + match *method { + ast::provided(ref m) => { + debug2!("provided {}", m.id); + self.exported_items.insert(m.id); + } + ast::required(ref m) => { + debug2!("required {}", m.id); + self.exported_items.insert(m.id); + } + } + } } - None => { - self.tcx.sess.span_bug(span, "method not found in \ - AST map?!"); + + // Default methods on traits are all public so long as the trait is + // public + ast::item_struct(ref def, _) if self.path_all_public => { + match def.ctor_id { + Some(id) => { self.exported_items.insert(id); } + None => {} + } } + + _ => {} } + + visit::walk_item(self, item, ()); + + self.path_all_public = orig_all_pub; } - // Returns true if the given local item is private and false otherwise. - fn local_item_is_private(&mut self, span: Span, item_id: NodeId) -> bool { - let mut f: &fn(NodeId) -> bool = |_| false; - f = |item_id| { - match self.tcx.items.find(&item_id) { - Some(&node_item(item, _)) => item.vis != public, - Some(&node_foreign_item(*)) => false, - Some(&node_method(method, impl_did, _)) => { - match method.vis { - private => true, - public => false, - inherited => f(impl_did.node) + fn visit_foreign_item(&mut self, a: @ast::foreign_item, _: ()) { + if self.path_all_public && a.vis == ast::public { + self.exported_items.insert(a.id); + } + } +} + +struct PrivacyVisitor<'self> { + tcx: ty::ctxt, + curitem: ast::NodeId, + + // Results of previous analyses necessary for privacy checking. + exported_items: &'self HashSet, + method_map: &'self method_map, + parents: &'self HashMap, + external_exports: resolve::ExternalExports, + last_private_map: resolve::LastPrivateMap, +} + +impl<'self> PrivacyVisitor<'self> { + // used when debugging + fn nodestr(&self, id: ast::NodeId) -> ~str { + ast_map::node_id_to_str(self.tcx.items, id, token::get_ident_interner()) + } + + // Determines whether the given definition is public from the point of view + // of the current item. + fn def_public(&self, did: ast::DefId) -> bool { + if !is_local(did) { + if self.external_exports.contains(&did) { + debug2!("privacy - {:?} was externally exported", did); + return true; + } + debug2!("privacy - is {:?} a public method", did); + return match self.tcx.methods.find(&did) { + Some(meth) => { + debug2!("privacy - well at least it's a method: {:?}", meth); + match meth.container { + ty::TraitContainer(id) => { + debug2!("privacy - recursing on trait {:?}", id); + self.def_public(id) + } + ty::ImplContainer(id) => { + match ty::impl_trait_ref(self.tcx, id) { + Some(t) => { + debug2!("privacy - impl of trait {:?}", id); + self.def_public(t.def_id) + } + None => { + debug2!("privacy - found a method {:?}", + meth.vis); + meth.vis == ast::public + } + } + } } } - Some(&node_trait_method(_, trait_did, _)) => f(trait_did.node), - Some(_) => { - self.tcx.sess.span_bug(span, - format!("local_item_is_private: item was \ - a {}?!", - ast_map::node_id_to_str( - self.tcx.items, - item_id, - token::get_ident_interner()))); - } None => { - self.tcx.sess.span_bug(span, "item not found in AST map?!"); + debug2!("privacy - nope, not even a method"); + false } + }; + } else if self.exported_items.contains(&did.node) { + debug2!("privacy - exported item {}", self.nodestr(did.node)); + return true; + } + + debug2!("privacy - local {:?} not public all the way down", did); + // return quickly for things in the same module + if self.parents.find(&did.node) == self.parents.find(&self.curitem) { + debug2!("privacy - same parent, we're done here"); + return true; + } + + // We now know that there is at least one private member between the + // destination and the root. + let mut closest_private_id = did.node; + loop { + debug2!("privacy - examining {}", self.nodestr(closest_private_id)); + let vis = match self.tcx.items.find(&closest_private_id) { + Some(&ast_map::node_item(it, _)) => it.vis, + Some(&ast_map::node_method(ref m, _, _)) => m.vis, + Some(&ast_map::node_foreign_item(_, _, v, _)) => v, + Some(&ast_map::node_variant(ref v, _, _)) => { + // sadly enum variants still inherit visibility, so only + // break out of this is explicitly private + if v.node.vis == ast::private { break } + ast::public // need to move up a level (to the enum) + } + _ => ast::public, + }; + if vis != ast::public { break } + closest_private_id = *self.parents.get(&closest_private_id); + + // If we reached the top, then we should have been public all the + // way down in the first place... + assert!(closest_private_id != ast::DUMMY_NODE_ID); + } + debug2!("privacy - closest priv {}", self.nodestr(closest_private_id)); + return self.private_accessible(closest_private_id); + } + + /// For a local private node in the AST, this function will determine + /// whether the node is accessible by the current module that iteration is + /// inside. + fn private_accessible(&self, id: ast::NodeId) -> bool { + let parent = *self.parents.get(&id); + debug2!("privacy - accessible parent {}", self.nodestr(parent)); + + // After finding `did`'s closest private member, we roll ourselves back + // to see if this private member's parent is anywhere in our ancestry. + // By the privacy rules, we can access all of our ancestor's private + // members, so that's why we test the parent, and not the did itself. + let mut cur = self.curitem; + loop { + debug2!("privacy - questioning {}", self.nodestr(cur)); + match cur { + // If the relevant parent is in our history, then we're allowed + // to look inside any of our ancestor's immediate private items, + // so this access is valid. + x if x == parent => return true, + + // If we've reached the root, then we couldn't access this item + // in the first place + ast::DUMMY_NODE_ID => return false, + + // Keep going up + _ => {} } - }; - f(item_id) + + cur = *self.parents.get(&cur); + } } - // Checks that a private field is in scope. + // Checks that a dereference of a univariant enum can occur. + fn check_variant(&self, span: Span, enum_id: ast::DefId) { + let variant_info = ty::enum_variants(self.tcx, enum_id)[0]; + if !self.def_public(variant_info.id) { + self.tcx.sess.span_err(span, "can only dereference enums \ + with a single, public variant"); + } + } + + // Checks that a field is in scope. // FIXME #6993: change type (and name) from Ident to Name fn check_field(&mut self, span: Span, id: ast::DefId, ident: ast::Ident) { let fields = ty::lookup_struct_fields(self.tcx, id); for field in fields.iter() { if field.name != ident.name { continue; } - if field.vis == private { + // public fields are public everywhere + if field.vis != ast::private { break } + if !is_local(field.id) || + !self.private_accessible(field.id.node) { self.tcx.sess.span_err(span, format!("field `{}` is private", token::ident_to_str(&ident))); } @@ -235,78 +376,59 @@ impl PrivacyVisitor { } // Given the ID of a method, checks to ensure it's in scope. - fn check_method_common(&mut self, span: Span, method_id: DefId, name: &Ident) { + fn check_static_method(&mut self, span: Span, method_id: ast::DefId, + name: &ast::Ident) { // If the method is a default method, we need to use the def_id of // the default implementation. - // Having to do this this is really unfortunate. - let method_id = ty::method(self.tcx, method_id).provided_source.unwrap_or(method_id); - - if method_id.crate == LOCAL_CRATE { - let is_private = self.method_is_private(span, method_id.node); - let container_id = ty::method(self.tcx, method_id).container_id(); - if is_private && - (container_id.crate != LOCAL_CRATE || - !self.privileged_items.iter().any(|x| x == &(container_id.node))) { - self.tcx.sess.span_err(span, - format!("method `{}` is private", - token::ident_to_str(name))); - } - } else { - let visibility = - csearch::get_item_visibility(self.tcx.sess.cstore, method_id); - if visibility != public { - self.tcx.sess.span_err(span, - format!("method `{}` is private", - token::ident_to_str(name))); - } + let method_id = ty::method(self.tcx, method_id).provided_source + .unwrap_or(method_id); + + if !self.def_public(method_id) { + debug2!("private: {:?}", method_id); + self.tcx.sess.span_err(span, format!("method `{}` is private", + token::ident_to_str(name))); } } - // Checks that a private path is in scope. - fn check_path(&mut self, span: Span, def: Def, path: &Path) { - debug2!("checking path"); - match def { - DefStaticMethod(method_id, _, _) => { - debug2!("found static method def, checking it"); - self.check_method_common(span, - method_id, - &path.segments.last().identifier) - } - DefFn(def_id, _) => { - if def_id.crate == LOCAL_CRATE { - if self.local_item_is_private(span, def_id.node) && - !self.privileged_items.iter().any(|x| x == &def_id.node) { - self.tcx.sess.span_err(span, - format!("function `{}` is private", - token::ident_to_str( - &path.segments - .last() - .identifier))); - } - //} else if csearch::get_item_visibility(self.tcx.sess.cstore, - // def_id) != public { - // self.tcx.sess.span_err(span, - // format!("function `{}` is private", - // token::ident_to_str( - // &path.segments - // .last() - // .identifier))); - } - // 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. + // Checks that a path is in scope. + fn check_path(&mut self, span: Span, path_id: ast::NodeId, path: &ast::Path) { + debug2!("privacy - path {}", self.nodestr(path_id)); + let ck = |tyname: &str| { + let last_private = *self.last_private_map.get(&path_id); + debug2!("privacy - {:?}", last_private); + let public = match last_private { + resolve::AllPublic => true, + resolve::DependsOn(def) => self.def_public(def), + }; + if !public { + debug2!("denying {:?}", path); + let name = token::ident_to_str(&path.segments.last() + .identifier); + self.tcx.sess.span_err(span, + format!("{} `{}` is private", tyname, name)); } + }; + match self.tcx.def_map.get_copy(&path_id) { + ast::DefStaticMethod(*) => ck("static method"), + ast::DefFn(*) => ck("function"), + ast::DefStatic(*) => ck("static"), + ast::DefVariant(*) => ck("variant"), + ast::DefTy(*) => ck("type"), + ast::DefTrait(*) => ck("trait"), + ast::DefStruct(*) => ck("struct"), + ast::DefMethod(_, Some(*)) => ck("trait method"), + ast::DefMethod(*) => ck("method"), + ast::DefMod(*) => ck("module"), _ => {} } } - // Checks that a private method is in scope. - fn check_method(&mut self, span: Span, origin: &method_origin, ident: ast::Ident) { + // Checks that a method is in scope. + fn check_method(&mut self, span: Span, origin: &method_origin, + ident: ast::Ident) { match *origin { method_static(method_id) => { - self.check_method_common(span, method_id, &ident) + self.check_static_method(span, method_id, &ident) } method_param(method_param { trait_id: trait_id, @@ -318,426 +440,376 @@ impl PrivacyVisitor { method_num: method_num, _ }) => { - if trait_id.crate == LOCAL_CRATE { - match self.tcx.items.find(&trait_id.node) { - Some(&node_item(item, _)) => { - match item.node { - item_trait(_, _, ref methods) => { - if method_num >= (*methods).len() { - self.tcx.sess.span_bug(span, - "method number out of range?!"); + if !self.def_public(trait_id) { + self.tcx.sess.span_err(span, "source trait is private"); + return; + } + match self.tcx.items.find(&trait_id.node) { + Some(&ast_map::node_item(item, _)) => { + match item.node { + ast::item_trait(_, _, ref methods) => { + match methods[method_num] { + ast::provided(ref method) => { + let def = ast::DefId { + node: method.id, + crate: trait_id.crate, + }; + if self.def_public(def) { return } + let msg = format!("method `{}` is \ + private", + token::ident_to_str( + &method.ident)); + self.tcx.sess.span_err(span, msg); } - match (*methods)[method_num] { - provided(method) - if method.vis == private && - !self.privileged_items.iter() - .any(|x| x == &(trait_id.node)) => { - self.tcx.sess.span_err(span, - format!("method `{}` is private", - token::ident_to_str(&method - .ident))); - } - provided(_) | required(_) => { - // Required methods can't be - // private. - } + ast::required(_) => { + // Required methods can't be private. } } - _ => { - self.tcx.sess.span_bug(span, "trait wasn't actually a trait?!"); - } } - } - Some(_) => { - self.tcx.sess.span_bug(span, "trait wasn't an item?!"); - } - None => { - self.tcx.sess.span_bug(span, - "trait item wasn't found in the AST map?!"); + _ => self.tcx.sess.span_bug(span, "trait wasn't \ + actually a trait?!"), } } - } else { - // FIXME #4732: External crates. + Some(_) => self.tcx.sess.span_bug(span, "trait wasn't an \ + item?!"), + None => self.tcx.sess.span_bug(span, "trait item wasn't \ + found in the AST \ + map?!"), } } } } -} - -impl<'self> Visitor> for PrivacyVisitor { - - fn visit_mod(&mut self, the_module:&_mod, _:Span, _:NodeId, - cx: Context<'self>) { - - let n_added = self.add_privileged_items(the_module.items); - visit::walk_mod(self, the_module, cx); - - do n_added.times { - ignore(self.privileged_items.pop()); + /// Validates all of the visibility qualifers placed on the item given. This + /// ensures that there are no extraneous qualifiers that don't actually do + /// anything. In theory these qualifiers wouldn't parse, but that may happen + /// later on down the road... + fn check_sane_privacy(&self, item: @ast::item) { + let tcx = self.tcx; + let check_inherited = |sp: Span, vis: ast::visibility, note: &str| { + if vis != ast::inherited { + tcx.sess.span_err(sp, "unnecessary visibility qualifier"); + if note.len() > 0 { + tcx.sess.span_note(sp, note); + } } - } - - fn visit_item(&mut self, item:@item, cx: Context<'self>) { - - // Do not check privacy inside items with the resolve_unexported - // attribute. This is used for the test runner. - if attr::contains_name(item.attrs, "!resolve_unexported") { - return; - } - - // Disallow unnecessary visibility qualifiers - check_sane_privacy(self.tcx, item); - - // Keep track of whether this item is available for export or not. - let orig_all_pub = self.path_all_public; + }; + let check_not_priv = |sp: Span, vis: ast::visibility, note: &str| { + if vis == ast::private { + tcx.sess.span_err(sp, "unnecessary `priv` qualifier"); + if note.len() > 0 { + tcx.sess.span_note(sp, note); + } + } + }; match item.node { - // impls/extern blocks do not break the "public chain" because they - // cannot have visibility qualifiers on them anyway - ast::item_impl(*) | ast::item_foreign_mod(*) => {} - - // Private by default, hence we only retain the "public chain" if - // `pub` is explicitly listed. - _ => { - self.path_all_public = orig_all_pub && item.vis == ast::public; + // implementations of traits don't need visibility qualifiers because + // that's controlled by having the trait in scope. + ast::item_impl(_, Some(*), _, ref methods) => { + check_inherited(item.span, item.vis, + "visibility qualifiers have no effect on trait \ + impls"); + for m in methods.iter() { + check_inherited(m.span, m.vis, ""); + } } - } - debug2!("public path at {}: {}", item.id, self.path_all_public); - if self.path_all_public { - debug2!("all the way public {}", item.id); - self.exported_items.insert(item.id); + ast::item_impl(_, _, _, ref methods) => { + check_inherited(item.span, item.vis, + "place qualifiers on individual methods instead"); + for i in methods.iter() { + check_not_priv(i.span, i.vis, "functions are private by \ + default"); + } + } + ast::item_foreign_mod(ref fm) => { + check_inherited(item.span, item.vis, + "place qualifiers on individual functions \ + instead"); + for i in fm.items.iter() { + check_not_priv(i.span, i.vis, "functions are private by \ + default"); + } + } - // All re-exported items in a module which is public should also be - // public (in terms of how they should get encoded) - match item.node { - ast::item_mod(*) => { - let (_, exp_map2) = cx; - match exp_map2.find(&item.id) { - Some(exports) => { - for export in exports.iter() { - if export.reexport && is_local(export.def_id) { - debug2!("found reexported {:?}", export); - let id = export.def_id.node; - self.exported_items.insert(id); - } + ast::item_enum(ref def, _) => { + for v in def.variants.iter() { + match v.node.vis { + ast::public => { + if item.vis == ast::public { + tcx.sess.span_err(v.span, "unnecessary `pub` \ + visibility"); + } + } + ast::private => { + if item.vis != ast::public { + tcx.sess.span_err(v.span, "unnecessary `priv` \ + visibility"); } } - None => {} + ast::inherited => {} } } - _ => {} } - } - - visit::walk_item(self, item, cx); - - self.path_all_public = orig_all_pub; - } - - fn visit_block(&mut self, block:&Block, cx: Context<'self>) { - // Gather up all the privileged items. - let mut n_added = 0; - for stmt in block.stmts.iter() { - match stmt.node { - StmtDecl(decl, _) => { - match decl.node { - DeclItem(item) => { - self.add_privileged_item(item, &mut n_added); - } - _ => {} + ast::item_struct(ref def, _) => { + for f in def.fields.iter() { + match f.node.kind { + ast::named_field(_, ast::public) => { + tcx.sess.span_err(f.span, "unnecessary `pub` \ + visibility"); } + ast::named_field(_, ast::private) => { + // Fields should really be private by default... + } + ast::named_field(*) | ast::unnamed_field => {} } - _ => {} } } - visit::walk_block(self, block, cx); + ast::item_trait(_, _, ref methods) => { + for m in methods.iter() { + match *m { + ast::provided(ref m) => { + check_inherited(m.span, m.vis, + "unnecessary visibility"); + } + ast::required(*) => {} + } + } + } - do n_added.times { - ignore(self.privileged_items.pop()); + ast::item_static(*) | + ast::item_fn(*) | ast::item_mod(*) | ast::item_ty(*) | + ast::item_mac(*) => { + check_not_priv(item.span, item.vis, "items are private by \ + default"); } + } + } +} + +impl<'self> Visitor<()> for PrivacyVisitor<'self> { + fn visit_item(&mut self, item: @ast::item, _: ()) { + // Do not check privacy inside items with the resolve_unexported + // attribute. This is used for the test runner. + if attr::contains_name(item.attrs, "!resolve_unexported") { + return; + } + + // Disallow unnecessary visibility qualifiers + self.check_sane_privacy(item); + let orig_curitem = self.curitem; + self.curitem = item.id; + visit::walk_item(self, item, ()); + self.curitem = orig_curitem; } - fn visit_expr(&mut self, expr:@Expr, cx: Context<'self>) { - let (method_map, _) = cx; - match expr.node { - ExprField(base, ident, _) => { - // Method calls are now a special syntactic form, - // so `a.b` should always be a field. - assert!(!method_map.contains_key(&expr.id)); - - // With type_autoderef, make sure we don't - // allow pointers to violate privacy - match ty::get(ty::type_autoderef(self.tcx, ty::expr_ty(self.tcx, - base))).sty { - ty_struct(id, _) - if id.crate != LOCAL_CRATE || !self.privileged_items.iter() - .any(|x| x == &(id.node)) => { - debug2!("(privacy checking) checking field access"); - self.check_field(expr.span, id, ident); - } - _ => {} - } + fn visit_expr(&mut self, expr: @ast::Expr, _: ()) { + match expr.node { + ast::ExprField(base, ident, _) => { + // Method calls are now a special syntactic form, + // so `a.b` should always be a field. + assert!(!self.method_map.contains_key(&expr.id)); + + // With type_autoderef, make sure we don't + // allow pointers to violate privacy + let t = ty::type_autoderef(self.tcx, + ty::expr_ty(self.tcx, base)); + match ty::get(t).sty { + ty::ty_struct(id, _) => self.check_field(expr.span, id, ident), + _ => {} } - ExprMethodCall(_, base, ident, _, _, _) => { - // Ditto - match ty::get(ty::type_autoderef(self.tcx, ty::expr_ty(self.tcx, - base))).sty { - ty_enum(id, _) | - ty_struct(id, _) - if id.crate != LOCAL_CRATE || - !self.privileged_items.iter().any(|x| x == &(id.node)) => { - match method_map.find(&expr.id) { - None => { - self.tcx.sess.span_bug(expr.span, - "method call not in \ - method map"); - } - Some(ref entry) => { - debug2!("(privacy checking) checking \ - impl method"); - self.check_method(expr.span, &entry.origin, ident); - } + } + ast::ExprMethodCall(_, base, ident, _, _, _) => { + // see above + let t = ty::type_autoderef(self.tcx, + ty::expr_ty(self.tcx, base)); + match ty::get(t).sty { + ty::ty_enum(_, _) | ty::ty_struct(_, _) => { + let entry = match self.method_map.find(&expr.id) { + None => { + self.tcx.sess.span_bug(expr.span, + "method call not in \ + method map"); } - } - _ => {} + Some(entry) => entry + }; + debug2!("(privacy checking) checking impl method"); + self.check_method(expr.span, &entry.origin, ident); } + _ => {} } - ExprPath(ref path) => { - self.check_path(expr.span, self.tcx.def_map.get_copy(&expr.id), path); - } - ExprStruct(_, ref fields, _) => { - match ty::get(ty::expr_ty(self.tcx, expr)).sty { - ty_struct(id, _) => { - if id.crate != LOCAL_CRATE || - !self.privileged_items.iter().any(|x| x == &(id.node)) { - for field in (*fields).iter() { - debug2!("(privacy checking) checking \ - field in struct literal"); - self.check_field(expr.span, id, field.ident); - } - } + } + ast::ExprPath(ref path) => { + self.check_path(expr.span, expr.id, path); + } + ast::ExprStruct(_, ref fields, _) => { + match ty::get(ty::expr_ty(self.tcx, expr)).sty { + ty::ty_struct(id, _) => { + for field in (*fields).iter() { + self.check_field(expr.span, id, field.ident); } - ty_enum(id, _) => { - if id.crate != LOCAL_CRATE || - !self.privileged_items.iter().any(|x| x == &(id.node)) { - match self.tcx.def_map.get_copy(&expr.id) { - DefVariant(_, variant_id, _) => { - for field in (*fields).iter() { - debug2!("(privacy checking) \ - checking field in \ - struct variant \ - literal"); - self.check_field(expr.span, variant_id, field.ident); - } - } - _ => { - self.tcx.sess.span_bug(expr.span, - "resolve didn't \ - map enum struct \ - constructor to a \ - variant def"); - } + } + ty::ty_enum(_, _) => { + match self.tcx.def_map.get_copy(&expr.id) { + ast::DefVariant(_, variant_id, _) => { + for field in fields.iter() { + self.check_field(expr.span, variant_id, + field.ident); } } - } - _ => { - self.tcx.sess.span_bug(expr.span, "struct expr \ - didn't have \ - struct type?!"); + _ => self.tcx.sess.span_bug(expr.span, + "resolve didn't \ + map enum struct \ + constructor to a \ + variant def"), } } + _ => self.tcx.sess.span_bug(expr.span, "struct expr \ + didn't have \ + struct type?!"), } - ExprUnary(_, ast::UnDeref, operand) => { - // In *e, we need to check that if e's type is an - // enum type t, then t's first variant is public or - // privileged. (We can assume it has only one variant - // since typeck already happened.) - match ty::get(ty::expr_ty(self.tcx, operand)).sty { - ty_enum(id, _) => { - if id.crate != LOCAL_CRATE || - !self.privileged_items.iter().any(|x| x == &(id.node)) { - self.check_variant(expr.span, id); - } - } - _ => { /* No check needed */ } + } + ast::ExprUnary(_, ast::UnDeref, operand) => { + // In *e, we need to check that if e's type is an + // enum type t, then t's first variant is public or + // privileged. (We can assume it has only one variant + // since typeck already happened.) + match ty::get(ty::expr_ty(self.tcx, operand)).sty { + ty::ty_enum(id, _) => { + self.check_variant(expr.span, id); } + _ => { /* No check needed */ } } - _ => {} } + _ => {} + } - visit::walk_expr(self, expr, cx); - + visit::walk_expr(self, expr, ()); } - fn visit_pat(&mut self, pattern:@Pat, cx: Context<'self>) { + fn visit_ty(&mut self, t: &ast::Ty, _: ()) { + match t.node { + ast::ty_path(ref path, _, id) => self.check_path(t.span, id, path), + _ => {} + } + visit::walk_ty(self, t, ()); + } - match pattern.node { - PatStruct(_, ref fields, _) => { - match ty::get(ty::pat_ty(self.tcx, pattern)).sty { - ty_struct(id, _) => { - if id.crate != LOCAL_CRATE || - !self.privileged_items.iter().any(|x| x == &(id.node)) { - for field in fields.iter() { - debug2!("(privacy checking) checking \ - struct pattern"); - self.check_field(pattern.span, id, field.ident); - } - } + fn visit_view_item(&mut self, a: &ast::view_item, _: ()) { + match a.node { + ast::view_item_extern_mod(*) => {} + ast::view_item_use(ref uses) => { + for vpath in uses.iter() { + match vpath.node { + ast::view_path_simple(_, ref path, id) | + ast::view_path_glob(ref path, id) => { + debug2!("privacy - glob/simple {}", id); + self.check_path(vpath.span, id, path); } - ty_enum(enum_id, _) => { - if enum_id.crate != LOCAL_CRATE || - !self.privileged_items.iter().any(|x| x == &enum_id.node) { - match self.tcx.def_map.find(&pattern.id) { - Some(&DefVariant(_, variant_id, _)) => { - for field in fields.iter() { - debug2!("(privacy checking) \ - checking field in \ - struct variant pattern"); - self.check_field(pattern.span, variant_id, field.ident); - } - } - _ => { - self.tcx.sess.span_bug(pattern.span, - "resolve didn't \ - map enum struct \ - pattern to a \ - variant def"); - } - } + ast::view_path_list(_, ref list, _) => { + for pid in list.iter() { + debug2!("privacy - list {}", pid.node.id); + let seg = ast::PathSegment { + identifier: pid.node.name, + lifetime: None, + types: opt_vec::Empty, + }; + let segs = ~[seg]; + let path = ast::Path { + global: false, + span: pid.span, + segments: segs, + }; + self.check_path(pid.span, pid.node.id, &path); } } - _ => { - self.tcx.sess.span_bug(pattern.span, - "struct pattern didn't have \ - struct type?!"); - } } } - _ => {} - } - - visit::walk_pat(self, pattern, cx); - } -} - -pub fn check_crate(tcx: ty::ctxt, - method_map: &method_map, - exp_map2: &ExportMap2, - crate: &ast::Crate) -> ExportedItems { - let privileged_items = @mut ~[]; - - let mut visitor = PrivacyVisitor { - tcx: tcx, - privileged_items: privileged_items, - exported_items: HashSet::new(), - path_all_public: true, // start out as public - }; - visit::walk_crate(&mut visitor, crate, (method_map, exp_map2)); - return visitor.exported_items; -} - -/// Validates all of the visibility qualifers placed on the item given. This -/// ensures that there are no extraneous qualifiers that don't actually do -/// anything. In theory these qualifiers wouldn't parse, but that may happen -/// later on down the road... -fn check_sane_privacy(tcx: ty::ctxt, item: @ast::item) { - let check_inherited = |sp: Span, vis: ast::visibility, note: &str| { - if vis != ast::inherited { - tcx.sess.span_err(sp, "unnecessary visibility qualifier"); - if note.len() > 0 { - tcx.sess.span_note(sp, note); - } - } - }; - let check_not_priv = |sp: Span, vis: ast::visibility, note: &str| { - if vis == ast::private { - tcx.sess.span_err(sp, "unnecessary `priv` qualifier"); - if note.len() > 0 { - tcx.sess.span_note(sp, note); - } - } - }; - match item.node { - // implementations of traits don't need visibility qualifiers because - // that's controlled by having the trait in scope. - ast::item_impl(_, Some(*), _, ref methods) => { - check_inherited(item.span, item.vis, - "visibility qualifiers have no effect on trait impls"); - for m in methods.iter() { - check_inherited(m.span, m.vis, ""); - } - } - - ast::item_impl(_, _, _, ref methods) => { - check_inherited(item.span, item.vis, - "place qualifiers on individual methods instead"); - for i in methods.iter() { - check_not_priv(i.span, i.vis, "functions are private by default"); - } - } - ast::item_foreign_mod(ref fm) => { - check_inherited(item.span, item.vis, - "place qualifiers on individual functions instead"); - for i in fm.items.iter() { - check_not_priv(i.span, i.vis, "functions are private by default"); } } + } - ast::item_enum(ref def, _) => { - for v in def.variants.iter() { - match v.node.vis { - ast::public => { - if item.vis == ast::public { - tcx.sess.span_err(v.span, "unnecessary `pub` \ - visibility"); + fn visit_pat(&mut self, pattern: @ast::Pat, _: ()) { + match pattern.node { + ast::PatStruct(_, ref fields, _) => { + match ty::get(ty::pat_ty(self.tcx, pattern)).sty { + ty::ty_struct(id, _) => { + for field in fields.iter() { + self.check_field(pattern.span, id, field.ident); } } - ast::private => { - if item.vis != ast::public { - tcx.sess.span_err(v.span, "unnecessary `priv` \ - visibility"); + ty::ty_enum(_, _) => { + match self.tcx.def_map.find(&pattern.id) { + Some(&ast::DefVariant(_, variant_id, _)) => { + for field in fields.iter() { + self.check_field(pattern.span, variant_id, + field.ident); + } + } + _ => self.tcx.sess.span_bug(pattern.span, + "resolve didn't \ + map enum struct \ + pattern to a \ + variant def"), } } - ast::inherited => {} + _ => self.tcx.sess.span_bug(pattern.span, + "struct pattern didn't have \ + struct type?!"), } } + _ => {} } - ast::item_struct(ref def, _) => { - for f in def.fields.iter() { - match f.node.kind { - ast::named_field(_, ast::public) => { - tcx.sess.span_err(f.span, "unnecessary `pub` \ - visibility"); - } - ast::named_field(_, ast::private) => { - // Fields should really be private by default... - } - ast::named_field(*) | ast::unnamed_field => {} - } - } - } + visit::walk_pat(self, pattern, ()); + } +} - ast::item_trait(_, _, ref methods) => { - for m in methods.iter() { - match *m { - ast::provided(ref m) => { - check_inherited(m.span, m.vis, - "unnecessary visibility"); - } - ast::required(*) => {} - } - } - } +pub fn check_crate(tcx: ty::ctxt, + method_map: &method_map, + exp_map2: &resolve::ExportMap2, + external_exports: resolve::ExternalExports, + last_private_map: resolve::LastPrivateMap, + crate: &ast::Crate) { + let mut parents = HashMap::new(); + let mut exported_items = HashSet::new(); + + // First, figure out who everyone's parent is + { + let mut visitor = ParentVisitor { + parents: &mut parents, + curparent: ast::DUMMY_NODE_ID, + }; + visit::walk_crate(&mut visitor, crate, ()); + } - ast::item_static(*) | - ast::item_fn(*) | ast::item_mod(*) | ast::item_ty(*) | - ast::item_mac(*) => { - check_not_priv(item.span, item.vis, "items are private by default"); - } + // Next, build up the list of all exported items from this crate + { + // Initialize the exported items with resolve's id for the "root crate" + // to resolve references to `super` leading to the root and such. + exported_items.insert(0); + let mut visitor = EmbargoVisitor { + exported_items: &mut exported_items, + exp_map2: exp_map2, + path_all_public: true, // start out as public + }; + visit::walk_crate(&mut visitor, crate, ()); + } + + // And then actually check the privacy of everything. + { + let mut visitor = PrivacyVisitor { + curitem: ast::DUMMY_NODE_ID, + tcx: tcx, + exported_items: &exported_items, + parents: &parents, + method_map: method_map, + external_exports: external_exports, + last_private_map: last_private_map, + }; + visit::walk_crate(&mut visitor, crate, ()); } } diff --git a/src/librustc/middle/resolve.rs b/src/librustc/middle/resolve.rs index 646c80a0a76e4..447f1075585c8 100644 --- a/src/librustc/middle/resolve.rs +++ b/src/librustc/middle/resolve.rs @@ -25,8 +25,6 @@ use syntax::ast::*; use syntax::ast; use syntax::ast_util::{def_id_of_def, local_def, mtwt_resolve}; use syntax::ast_util::{path_to_ident, walk_pat, trait_method_to_ty_method}; -use syntax::ast_util::{Privacy, Public, Private}; -use syntax::ast_util::{variant_visibility_to_privacy, visibility_to_privacy}; use syntax::attr; use syntax::parse::token; use syntax::parse::token::{ident_interner, interner_get}; @@ -71,6 +69,27 @@ pub struct Export2 { reexport: bool, // Whether this is a reexport. } +// This set contains all exported definitions from external crates. The set does +// not contain any entries from local crates. +pub type ExternalExports = HashSet; + +// XXX: dox +pub type LastPrivateMap = HashMap; + +pub enum LastPrivate { + AllPublic, + DependsOn(DefId), +} + +impl LastPrivate { + fn or(self, other: LastPrivate) -> LastPrivate { + match (self, other) { + (me, AllPublic) => me, + (_, other) => other, + } + } +} + #[deriving(Eq)] enum PatternBindingMode { RefutableMode, @@ -119,8 +138,8 @@ impl NamespaceResult { enum NameDefinition { NoNameDefinition, //< The name was unbound. - ChildNameDefinition(Def), //< The name identifies an immediate child. - ImportNameDefinition(Def) //< The name identifies an import. + ChildNameDefinition(Def, LastPrivate), //< The name identifies an immediate child. + ImportNameDefinition(Def, LastPrivate) //< The name identifies an import. } #[deriving(Eq)] @@ -275,21 +294,13 @@ enum NameSearchType { ImportSearch, /// We're doing a name search in order to resolve a path type, a path - /// expression, or a path pattern. We can select public or private - /// names. - /// - /// XXX: This should be ripped out of resolve and handled later, in - /// the privacy checking phase. - PathPublicOrPrivateSearch, - - /// We're doing a name search in order to resolve a path type, a path - /// expression, or a path pattern. Allow only public names to be selected. - PathPublicOnlySearch, + /// expression, or a path pattern. + PathSearch, } enum BareIdentifierPatternResolution { - FoundStructOrEnumVariant(Def), - FoundConst(Def), + FoundStructOrEnumVariant(Def, LastPrivate), + FoundConst(Def, LastPrivate), BareIdentifierPatternUnresolved } @@ -323,26 +334,26 @@ impl Rib { /// One import directive. struct ImportDirective { - privacy: Privacy, module_path: ~[Ident], subclass: @ImportDirectiveSubclass, span: Span, id: NodeId, + is_public: bool, // see note in ImportResolution about how to use this } impl ImportDirective { - fn new(privacy: Privacy, - module_path: ~[Ident], - subclass: @ImportDirectiveSubclass, - span: Span, - id: NodeId) - -> ImportDirective { + fn new(module_path: ~[Ident], + subclass: @ImportDirectiveSubclass, + span: Span, + id: NodeId, + is_public: bool) + -> ImportDirective { ImportDirective { - privacy: privacy, module_path: module_path, subclass: subclass, span: span, - id: id + id: id, + is_public: is_public, } } } @@ -366,9 +377,11 @@ impl Target { /// An ImportResolution represents a particular `use` directive. struct ImportResolution { - /// The privacy of this `use` directive (whether it's `use` or - /// `pub use`. - privacy: Privacy, + /// Whether this resolution came from a `use` or a `pub use`. Note that this + /// should *not* be used whenever resolution is being performed, this is + /// only looked at for glob imports statements currently. Privacy testing + /// occurs during a later phase of compilation. + is_public: bool, // The number of outstanding references to this name. When this reaches // zero, outside modules can count on the targets being correct. Before @@ -389,15 +402,14 @@ struct ImportResolution { } impl ImportResolution { - fn new(privacy: Privacy, - id: NodeId) -> ImportResolution { + fn new(id: NodeId, is_public: bool) -> ImportResolution { ImportResolution { - privacy: privacy, type_id: id, value_id: id, outstanding_references: 0, value_target: None, type_target: None, + is_public: is_public, } } @@ -439,6 +451,7 @@ struct Module { parent_link: ParentLink, def_id: Option, kind: ModuleKind, + is_public: bool, children: @mut HashMap, imports: @mut ~[@ImportDirective], @@ -480,14 +493,16 @@ struct Module { impl Module { fn new(parent_link: ParentLink, - def_id: Option, - kind: ModuleKind, - external: bool) + def_id: Option, + kind: ModuleKind, + external: bool, + is_public: bool) -> Module { Module { parent_link: parent_link, def_id: def_id, kind: kind, + is_public: is_public, children: @mut HashMap::new(), imports: @mut ~[], external_module_children: @mut HashMap::new(), @@ -507,7 +522,7 @@ impl Module { // Records a possibly-private type definition. struct TypeNsDef { - privacy: Privacy, + is_public: bool, // see note in ImportResolution about how to use this module_def: Option<@mut Module>, type_def: Option, type_span: Option @@ -515,7 +530,7 @@ struct TypeNsDef { // Records a possibly-private value definition. struct ValueNsDef { - privacy: Privacy, + is_public: bool, // see note in ImportResolution about how to use this def: Def, value_span: Option, } @@ -537,18 +552,19 @@ enum TraitReferenceType { impl NameBindings { /// Creates a new module in this set of name bindings. fn define_module(&mut self, - privacy: Privacy, - parent_link: ParentLink, - def_id: Option, - kind: ModuleKind, - external: bool, - sp: Span) { + parent_link: ParentLink, + def_id: Option, + kind: ModuleKind, + external: bool, + is_public: bool, + sp: Span) { // Merges the module with the existing type def or creates a new one. - let module_ = @mut Module::new(parent_link, def_id, kind, external); + let module_ = @mut Module::new(parent_link, def_id, kind, external, + is_public); match self.type_def { None => { self.type_def = Some(TypeNsDef { - privacy: privacy, + is_public: is_public, module_def: Some(module_), type_def: None, type_span: Some(sp) @@ -556,7 +572,7 @@ impl NameBindings { } Some(type_def) => { self.type_def = Some(TypeNsDef { - privacy: privacy, + is_public: is_public, module_def: Some(module_), type_span: Some(sp), type_def: type_def.type_def @@ -567,17 +583,18 @@ impl NameBindings { /// Sets the kind of the module, creating a new one if necessary. fn set_module_kind(&mut self, - privacy: Privacy, - parent_link: ParentLink, - def_id: Option, - kind: ModuleKind, - external: bool, - _sp: Span) { + parent_link: ParentLink, + def_id: Option, + kind: ModuleKind, + external: bool, + is_public: bool, + _sp: Span) { match self.type_def { None => { - let module = @mut Module::new(parent_link, def_id, kind, external); + let module = @mut Module::new(parent_link, def_id, kind, + external, is_public); self.type_def = Some(TypeNsDef { - privacy: privacy, + is_public: is_public, module_def: Some(module), type_def: None, type_span: None, @@ -587,11 +604,12 @@ impl NameBindings { match type_def.module_def { None => { let module = @mut Module::new(parent_link, - def_id, - kind, - external); + def_id, + kind, + external, + is_public); self.type_def = Some(TypeNsDef { - privacy: privacy, + is_public: is_public, module_def: Some(module), type_def: type_def.type_def, type_span: None, @@ -604,31 +622,32 @@ impl NameBindings { } /// Records a type definition. - fn define_type(&mut self, privacy: Privacy, def: Def, sp: Span) { + fn define_type(&mut self, def: Def, sp: Span, is_public: bool) { // Merges the type with the existing type def or creates a new one. match self.type_def { None => { self.type_def = Some(TypeNsDef { - privacy: privacy, module_def: None, type_def: Some(def), - type_span: Some(sp) + type_span: Some(sp), + is_public: is_public, }); } Some(type_def) => { self.type_def = Some(TypeNsDef { - privacy: privacy, type_def: Some(def), type_span: Some(sp), - module_def: type_def.module_def + module_def: type_def.module_def, + is_public: is_public, }); } } } /// Records a value definition. - fn define_value(&mut self, privacy: Privacy, def: Def, sp: Span) { - self.value_def = Some(ValueNsDef { privacy: privacy, def: def, value_span: Some(sp) }); + fn define_value(&mut self, def: Def, sp: Span, is_public: bool) { + self.value_def = Some(ValueNsDef { def: def, value_span: Some(sp), + is_public: is_public }); } /// Returns the module node if applicable. @@ -663,12 +682,10 @@ impl NameBindings { fn defined_in_public_namespace(&self, namespace: Namespace) -> bool { match namespace { TypeNS => match self.type_def { - Some(def) => def.privacy != Private, - None => false + Some(def) => def.is_public, None => false }, ValueNS => match self.value_def { - Some(def) => def.privacy != Private, - None => false + Some(def) => def.is_public, None => false } } } @@ -705,24 +722,6 @@ impl NameBindings { } } - fn privacy_for_namespace(&self, namespace: Namespace) - -> Option { - match namespace { - TypeNS => { - match self.type_def { - None => None, - Some(ref type_def) => Some((*type_def).privacy) - } - } - ValueNS => { - match self.value_def { - None => None, - Some(value_def) => Some(value_def.privacy) - } - } - } - } - fn span_for_namespace(&self, namespace: Namespace) -> Option { if self.defined_in_namespace(namespace) { match namespace { @@ -800,15 +799,15 @@ fn namespace_error_to_str(ns: NamespaceError) -> &'static str { } fn Resolver(session: Session, - lang_items: LanguageItems, - crate_span: Span) -> Resolver { + lang_items: LanguageItems, + crate_span: Span) -> Resolver { let graph_root = @mut NameBindings(); - graph_root.define_module(Public, - NoParentLink, + graph_root.define_module(NoParentLink, Some(DefId { crate: 0, node: 0 }), NormalModuleKind, false, + true, crate_span); let current_module = graph_root.get_module(); @@ -846,6 +845,8 @@ fn Resolver(session: Session, export_map2: @mut HashMap::new(), trait_map: HashMap::new(), used_imports: HashSet::new(), + external_exports: HashSet::new(), + last_private: HashMap::new(), emit_errors: true, intr: session.intr() @@ -903,6 +904,8 @@ struct Resolver { def_map: DefMap, export_map2: ExportMap2, trait_map: TraitMap, + external_exports: ExternalExports, + last_private: LastPrivateMap, // Whether or not to print error messages. Can be set to true // when getting additional info for error message suggestions, @@ -1161,7 +1164,7 @@ impl Resolver { { let ident = item.ident; let sp = item.span; - let privacy = visibility_to_privacy(item.vis); + let is_public = item.vis == ast::public; match item.node { item_mod(*) => { @@ -1170,11 +1173,11 @@ impl Resolver { let parent_link = self.get_parent_link(new_parent, ident); let def_id = DefId { crate: 0, node: item.id }; - name_bindings.define_module(privacy, - parent_link, + name_bindings.define_module(parent_link, Some(def_id), NormalModuleKind, false, + item.vis == ast::public, sp); ModuleReducedGraphParent(name_bindings.get_module()) @@ -1190,11 +1193,11 @@ impl Resolver { let parent_link = self.get_parent_link(new_parent, ident); let def_id = DefId { crate: 0, node: item.id }; - name_bindings.define_module(privacy, - parent_link, + name_bindings.define_module(parent_link, Some(def_id), ExternModuleKind, false, + true, sp); ModuleReducedGraphParent(name_bindings.get_module()) @@ -1213,7 +1216,7 @@ impl Resolver { let mutbl = m == ast::MutMutable; name_bindings.define_value - (privacy, DefStatic(local_def(item.id), mutbl), sp); + (DefStatic(local_def(item.id), mutbl), sp, is_public); parent } item_fn(_, purity, _, _, _) => { @@ -1221,7 +1224,7 @@ impl Resolver { self.add_child(ident, parent, ForbidDuplicateValues, sp); let def = DefFn(local_def(item.id), purity); - name_bindings.define_value(privacy, def, sp); + name_bindings.define_value(def, sp, is_public); new_parent } @@ -1231,7 +1234,7 @@ impl Resolver { self.add_child(ident, parent, ForbidDuplicateTypes, sp); name_bindings.define_type - (privacy, DefTy(local_def(item.id)), sp); + (DefTy(local_def(item.id)), sp, is_public); parent } @@ -1240,16 +1243,14 @@ impl Resolver { self.add_child(ident, parent, ForbidDuplicateTypes, sp); name_bindings.define_type - (privacy, DefTy(local_def(item.id)), sp); + (DefTy(local_def(item.id)), sp, is_public); for variant in (*enum_definition).variants.iter() { self.build_reduced_graph_for_variant( variant, local_def(item.id), - // inherited => privacy of the enum item - variant_visibility_to_privacy(variant.node.vis, - privacy == Public), - new_parent); + new_parent, + is_public); } parent } @@ -1265,12 +1266,13 @@ impl Resolver { let (name_bindings, new_parent) = self.add_child(ident, parent, forbid, sp); // Define a name in the type namespace. - name_bindings.define_type(privacy, DefTy(local_def(item.id)), sp); + name_bindings.define_type(DefTy(local_def(item.id)), sp, is_public); // If this is a newtype or unit-like struct, define a name // in the value namespace as well do ctor_id.while_some |cid| { - name_bindings.define_value(privacy, DefStruct(local_def(cid)), sp); + name_bindings.define_value(DefStruct(local_def(cid)), sp, + is_public); None } @@ -1316,11 +1318,11 @@ impl Resolver { let parent_link = self.get_parent_link(new_parent, ident); let def_id = local_def(item.id); - name_bindings.define_module(Public, - parent_link, + name_bindings.define_module(parent_link, Some(def_id), ImplModuleKind, false, + true, sp); ModuleReducedGraphParent( @@ -1353,9 +1355,10 @@ impl Resolver { } }; - method_name_bindings.define_value(Public, - def, - method.span); + let is_public = method.vis == ast::public; + method_name_bindings.define_value(def, + method.span, + is_public); } } _ => {} @@ -1372,11 +1375,11 @@ impl Resolver { // Add all the methods within to a new module. let parent_link = self.get_parent_link(parent, ident); - name_bindings.define_module(privacy, - parent_link, + name_bindings.define_module(parent_link, Some(local_def(item.id)), TraitModuleKind, false, + item.vis == ast::public, sp); let module_parent = ModuleReducedGraphParent(name_bindings. get_module()); @@ -1408,7 +1411,7 @@ impl Resolver { module_parent, ForbidDuplicateValues, ty_m.span); - method_name_bindings.define_value(Public, def, ty_m.span); + method_name_bindings.define_value(def, ty_m.span, true); // Add it to the trait info if not static. match ty_m.explicit_self.node { @@ -1430,7 +1433,7 @@ impl Resolver { } } - name_bindings.define_type(privacy, DefTrait(def_id), sp); + name_bindings.define_type(DefTrait(def_id), sp, is_public); new_parent } @@ -1443,35 +1446,30 @@ impl Resolver { // Constructs the reduced graph for one variant. Variants exist in the // type and/or value namespaces. fn build_reduced_graph_for_variant(&mut self, - variant: &variant, - item_id: DefId, - parent_privacy: Privacy, - parent: ReducedGraphParent) { + variant: &variant, + item_id: DefId, + parent: ReducedGraphParent, + parent_public: bool) { let ident = variant.node.name; - - let privacy = - match variant.node.vis { - public => Public, - private => Private, - inherited => parent_privacy - }; + // XXX: this is unfortunate to have to do this privacy calculation + // here. This should be living in middle::privacy, but it's + // necessary to keep around in some form becaues of glob imports... + let is_public = parent_public && variant.node.vis != ast::private; match variant.node.kind { tuple_variant_kind(_) => { let (child, _) = self.add_child(ident, parent, ForbidDuplicateValues, variant.span); - child.define_value(privacy, - DefVariant(item_id, - local_def(variant.node.id), false), - variant.span); + child.define_value(DefVariant(item_id, + local_def(variant.node.id), false), + variant.span, is_public); } struct_variant_kind(_) => { let (child, _) = self.add_child(ident, parent, ForbidDuplicateTypesAndValues, variant.span); - child.define_type(privacy, - DefVariant(item_id, - local_def(variant.node.id), true), - variant.span); + child.define_type(DefVariant(item_id, + local_def(variant.node.id), true), + variant.span, is_public); self.structs.insert(local_def(variant.node.id)); } } @@ -1482,7 +1480,6 @@ impl Resolver { fn build_reduced_graph_for_view_item(&mut self, view_item: &view_item, parent: ReducedGraphParent) { - let privacy = visibility_to_privacy(view_item.vis); match view_item.node { view_item_use(ref view_paths) => { for view_path in view_paths.iter() { @@ -1515,39 +1512,40 @@ impl Resolver { // Build up the import directives. let module_ = self.get_module_from_parent(parent); + let is_public = view_item.vis == ast::public; match view_path.node { view_path_simple(binding, ref full_path, id) => { let source_ident = full_path.segments.last().identifier; let subclass = @SingleImport(binding, source_ident); - self.build_import_directive(privacy, - module_, + self.build_import_directive(module_, module_path, subclass, view_path.span, - id); + id, + is_public); } view_path_list(_, ref source_idents, _) => { for source_ident in source_idents.iter() { let name = source_ident.node.name; let subclass = @SingleImport(name, name); self.build_import_directive( - privacy, module_, module_path.clone(), subclass, source_ident.span, - source_ident.node.id); + source_ident.node.id, + is_public); } } view_path_glob(_, id) => { - self.build_import_directive(privacy, - module_, + self.build_import_directive(module_, module_path, @GlobImport, view_path.span, - id); + id, + is_public); } } } @@ -1559,12 +1557,14 @@ impl Resolver { node_id) { Some(crate_id) => { let def_id = DefId { crate: crate_id, node: 0 }; + self.external_exports.insert(def_id); let parent_link = ModuleParentLink (self.get_module_from_parent(parent), name); let external_module = @mut Module::new(parent_link, Some(def_id), NormalModuleKind, - false); + false, + true); parent.external_module_children.insert( name.name, @@ -1586,6 +1586,7 @@ impl Resolver { f: &fn(&mut Resolver, ReducedGraphParent)) { let name = foreign_item.ident; + let is_public = foreign_item.vis == ast::public; let (name_bindings, new_parent) = self.add_child(name, parent, ForbidDuplicateValues, foreign_item.span); @@ -1593,7 +1594,7 @@ impl Resolver { match foreign_item.node { foreign_item_fn(_, ref generics) => { let def = DefFn(local_def(foreign_item.id), unsafe_fn); - name_bindings.define_value(Public, def, foreign_item.span); + name_bindings.define_value(def, foreign_item.span, is_public); do self.with_type_parameter_rib( HasTypeParameters( @@ -1604,7 +1605,7 @@ impl Resolver { } foreign_item_static(_, m) => { let def = DefStatic(local_def(foreign_item.id), m); - name_bindings.define_value(Public, def, foreign_item.span); + name_bindings.define_value(def, foreign_item.span, is_public); f(self, new_parent) } @@ -1628,6 +1629,7 @@ impl Resolver { BlockParentLink(parent_module, block_id), None, AnonymousModuleKind, + false, false); parent_module.anonymous_children.insert(block_id, new_module); ModuleReducedGraphParent(new_module) @@ -1638,15 +1640,18 @@ impl Resolver { fn handle_external_def(&mut self, def: Def, - visibility: ast::visibility, + vis: visibility, child_name_bindings: @mut NameBindings, final_ident: &str, ident: Ident, new_parent: ReducedGraphParent) { - let privacy = visibility_to_privacy(visibility); debug2!("(building reduced graph for \ external crate) building external def, priv {:?}", - privacy); + vis); + let is_public = vis == ast::public; + if is_public { + self.external_exports.insert(def_id_of_def(def)); + } match def { DefMod(def_id) | DefForeignMod(def_id) | DefStruct(def_id) | DefTy(def_id) => { @@ -1662,11 +1667,11 @@ impl Resolver { {}", final_ident); let parent_link = self.get_parent_link(new_parent, ident); - child_name_bindings.define_module(privacy, - parent_link, + child_name_bindings.define_module(parent_link, Some(def_id), NormalModuleKind, true, + is_public, dummy_sp()); } } @@ -1681,20 +1686,20 @@ impl Resolver { variant {}", final_ident); // We assume the parent is visible, or else we wouldn't have seen - // it. - let privacy = variant_visibility_to_privacy(visibility, true); + // it. Also variants are public-by-default if the parent was also + // public. + let is_public = vis != ast::private; if is_struct { - child_name_bindings.define_type(privacy, def, dummy_sp()); + child_name_bindings.define_type(def, dummy_sp(), is_public); self.structs.insert(variant_id); - } - else { - child_name_bindings.define_value(privacy, def, dummy_sp()); + } else { + child_name_bindings.define_value(def, dummy_sp(), is_public); } } DefFn(*) | DefStaticMethod(*) | DefStatic(*) => { debug2!("(building reduced graph for external \ crate) building value (fn/static) {}", final_ident); - child_name_bindings.define_value(privacy, def, dummy_sp()); + child_name_bindings.define_value(def, dummy_sp(), is_public); } DefTrait(def_id) => { debug2!("(building reduced graph for external \ @@ -1720,6 +1725,9 @@ impl Resolver { if explicit_self != sty_static { interned_method_names.insert(method_name.name); } + if is_public { + self.external_exports.insert(method_def_id); + } } for name in interned_method_names.iter() { if !self.method_map.contains_key(name) { @@ -1731,35 +1739,37 @@ impl Resolver { } } - child_name_bindings.define_type(privacy, def, dummy_sp()); + child_name_bindings.define_type(def, dummy_sp(), is_public); // Define a module if necessary. let parent_link = self.get_parent_link(new_parent, ident); - child_name_bindings.set_module_kind(privacy, - parent_link, + child_name_bindings.set_module_kind(parent_link, Some(def_id), TraitModuleKind, true, + is_public, dummy_sp()) } DefTy(_) => { debug2!("(building reduced graph for external \ crate) building type {}", final_ident); - child_name_bindings.define_type(privacy, def, dummy_sp()); + child_name_bindings.define_type(def, dummy_sp(), is_public); } DefStruct(def_id) => { debug2!("(building reduced graph for external \ crate) building type and value for {}", final_ident); - child_name_bindings.define_type(privacy, def, dummy_sp()); + child_name_bindings.define_type(def, dummy_sp(), is_public); if get_struct_fields(self.session.cstore, def_id).len() == 0 { - child_name_bindings.define_value(privacy, def, dummy_sp()); + child_name_bindings.define_value(def, dummy_sp(), is_public); } self.structs.insert(def_id); } DefMethod(*) => { - // Ignored; handled elsewhere. + debug2!("(building reduced graph for external crate) \ + ignoring {:?}", def); + // Ignored; handled elsewhere. } DefSelf(*) | DefArg(*) | DefLocal(*) | DefPrimTy(*) | DefTyParam(*) | DefBinding(*) | @@ -1854,11 +1864,11 @@ impl Resolver { self.get_parent_link(new_parent, final_ident); child_name_bindings.define_module( - Public, parent_link, Some(def), ImplModuleKind, true, + true, dummy_sp()); type_module = child_name_bindings. @@ -1886,10 +1896,9 @@ impl Resolver { static_method_info.def_id, static_method_info.purity); - let p = visibility_to_privacy( - static_method_info.vis); method_name_bindings.define_value( - p, def, dummy_sp()); + def, dummy_sp(), + visibility == ast::public); } } @@ -1956,14 +1965,15 @@ impl Resolver { /// Creates and adds an import directive to the given module. fn build_import_directive(&mut self, - privacy: Privacy, - module_: @mut Module, - module_path: ~[Ident], - subclass: @ImportDirectiveSubclass, - span: Span, - id: NodeId) { - let directive = @ImportDirective::new(privacy, module_path, - subclass, span, id); + module_: @mut Module, + module_path: ~[Ident], + subclass: @ImportDirectiveSubclass, + span: Span, + id: NodeId, + is_public: bool) { + let directive = @ImportDirective::new(module_path, + subclass, span, id, + is_public); module_.imports.push(directive); // Bump the reference count on the name. Or, if this is a glob, set @@ -1972,8 +1982,7 @@ impl Resolver { match *subclass { SingleImport(target, _) => { debug2!("(building import directive) building import \ - directive: privacy {:?} {}::{}", - privacy, + directive: {}::{}", self.idents_to_str(directive.module_path), self.session.str_of(target)); @@ -1984,13 +1993,12 @@ impl Resolver { resolution.outstanding_references += 1; // the source of this name is different now - resolution.privacy = privacy; resolution.type_id = id; resolution.value_id = id; } None => { debug2!("(building import directive) creating new"); - let resolution = @mut ImportResolution::new(privacy, id); + let resolution = @mut ImportResolution::new(id, is_public); resolution.outstanding_references = 1; module_.import_resolutions.insert(target.name, resolution); } @@ -2153,8 +2161,8 @@ impl Resolver { /// currently-unresolved imports, or success if we know the name exists. /// If successful, the resolved bindings are written into the module. fn resolve_import_for_module(&mut self, - module_: @mut Module, - import_directive: @ImportDirective) + module_: @mut Module, + import_directive: @ImportDirective) -> ResolveResult<()> { let mut resolution_result = Failed; let module_path = &import_directive.module_path; @@ -2165,9 +2173,9 @@ impl Resolver { self.module_to_str(module_)); // First, resolve the module path for the directive, if necessary. - let containing_module = if module_path.len() == 0 { + let container = if module_path.len() == 0 { // Use the crate root. - Some(self.graph_root.get_module()) + Some((self.graph_root.get_module(), AllPublic)) } else { match self.resolve_module_path(module_, *module_path, @@ -2180,13 +2188,13 @@ impl Resolver { resolution_result = Indeterminate; None } - Success(containing_module) => Some(containing_module), + Success(container) => Some(container), } }; - match containing_module { + match container { None => {} - Some(containing_module) => { + Some((containing_module, lp)) => { // We found the module that the target is contained // within. Attempt to resolve the import within it. @@ -2197,15 +2205,16 @@ impl Resolver { containing_module, target, source, - import_directive); + import_directive, + lp); } GlobImport => { - let privacy = import_directive.privacy; resolution_result = - self.resolve_glob_import(privacy, - module_, + self.resolve_glob_import(module_, containing_module, - import_directive.id); + import_directive.id, + import_directive.is_public, + lp); } } } @@ -2246,7 +2255,7 @@ impl Resolver { -> NameBindings { NameBindings { type_def: Some(TypeNsDef { - privacy: Public, + is_public: false, module_def: Some(module), type_def: None, type_span: None @@ -2256,18 +2265,21 @@ impl Resolver { } fn resolve_single_import(&mut self, - module_: @mut Module, - containing_module: @mut Module, - target: Ident, - source: Ident, - directive: &ImportDirective) + module_: @mut Module, + containing_module: @mut Module, + target: Ident, + source: Ident, + directive: &ImportDirective, + lp: LastPrivate) -> ResolveResult<()> { debug2!("(resolving single import) resolving `{}` = `{}::{}` from \ - `{}`", + `{}` id {}, last private {:?}", self.session.str_of(target), self.module_to_str(containing_module), self.session.str_of(source), - self.module_to_str(module_)); + self.module_to_str(module_), + directive.id, + lp); // We need to resolve both namespaces for this to succeed. // @@ -2295,6 +2307,7 @@ impl Resolver { // Unless we managed to find a result in both namespaces (unlikely), // search imports as well. + let mut used_reexport = false; match (value_result, type_result) { (BoundResult(*), BoundResult(*)) => {} // Continue. _ => { @@ -2337,7 +2350,7 @@ impl Resolver { // Import resolutions must be declared with "pub" // in order to be exported. - if import_resolution.privacy == Private { + if !import_resolution.is_public { return UnboundResult; } @@ -2360,11 +2373,14 @@ impl Resolver { if value_result.is_unknown() { value_result = get_binding(self, *import_resolution, ValueNS); + used_reexport = import_resolution.is_public; } if type_result.is_unknown() { type_result = get_binding(self, *import_resolution, TypeNS); + used_reexport = import_resolution.is_public; } + } Some(_) => { // The import is unresolved. Bail out. @@ -2378,6 +2394,7 @@ impl Resolver { // If we didn't find a result in the type namespace, search the // external modules. + let mut used_public = false; match type_result { BoundResult(*) => {} _ => { @@ -2390,6 +2407,7 @@ impl Resolver { *module); type_result = BoundResult(containing_module, name_bindings); + used_public = true; } } } @@ -2405,6 +2423,7 @@ impl Resolver { import_resolution.value_target = Some(Target::new(target_module, name_bindings)); import_resolution.value_id = directive.id; + used_public = name_bindings.defined_in_public_namespace(ValueNS); } UnboundResult => { /* Continue. */ } UnknownResult => { @@ -2418,6 +2437,7 @@ impl Resolver { import_resolution.type_target = Some(Target::new(target_module, name_bindings)); import_resolution.type_id = directive.id; + used_public = name_bindings.defined_in_public_namespace(TypeNS); } UnboundResult => { /* Continue. */ } UnknownResult => { @@ -2425,61 +2445,16 @@ impl Resolver { } } - let i = import_resolution; - let mut resolve_fail = false; - let mut priv_fail = false; - match (i.value_target, i.type_target) { - // If this name wasn't found in either namespace, it's definitely - // unresolved. - (None, None) => { resolve_fail = true; } - // If it's private, it's also unresolved. - (Some(t), None) | (None, Some(t)) => { - let bindings = &mut *t.bindings; - match bindings.type_def { - Some(ref type_def) => { - if type_def.privacy == Private { - priv_fail = true; - } - } - _ => () - } - match bindings.value_def { - Some(ref value_def) => { - if value_def.privacy == Private { - priv_fail = true; - } - } - _ => () - } - } - // It's also an error if there's both a type and a value with this - // name, but both are private - (Some(val), Some(ty)) => { - match (val.bindings.value_def, ty.bindings.value_def) { - (Some(ref value_def), Some(ref type_def)) => - if value_def.privacy == Private - && type_def.privacy == Private { - priv_fail = true; - }, - _ => () - } - } - } - - let span = directive.span; - if resolve_fail { - let msg = format!("unresolved import: there is no `{}` in `{}`", + if import_resolution.value_target.is_none() && + import_resolution.type_target.is_none() { + let msg = format!("unresolved import: there is no \ + `{}` in `{}`", self.session.str_of(source), self.module_to_str(containing_module)); - self.resolve_error(span, msg); - return Failed; - } else if priv_fail { - let msg = format!("unresolved import: found `{}` in `{}` but it is \ - private", self.session.str_of(source), - self.module_to_str(containing_module)); - self.resolve_error(span, msg); + self.resolve_error(directive.span, msg); return Failed; } + let used_public = used_reexport || used_public; assert!(import_resolution.outstanding_references >= 1); import_resolution.outstanding_references -= 1; @@ -2487,19 +2462,23 @@ impl Resolver { // record what this import resolves to for later uses in documentation, // this may resolve to either a value or a type, but for documentation // purposes it's good enough to just favor one over the other. - match i.value_target { + match import_resolution.value_target { Some(target) => { - self.def_map.insert(i.value_id, - target.bindings.value_def.get_ref().def); + let def = target.bindings.def_for_namespace(ValueNS).unwrap(); + self.def_map.insert(directive.id, def); + let did = def_id_of_def(def); + self.last_private.insert(directive.id, + if used_public {lp} else {DependsOn(did)}); } None => {} } - match i.type_target { + match import_resolution.type_target { Some(target) => { - match target.bindings.type_def.get_ref().type_def { - Some(def) => { self.def_map.insert(i.type_id, def); } - None => {} - } + let def = target.bindings.def_for_namespace(TypeNS).unwrap(); + self.def_map.insert(directive.id, def); + let did = def_id_of_def(def); + self.last_private.insert(directive.id, + if used_public {lp} else {DependsOn(did)}); } None => {} } @@ -2512,15 +2491,16 @@ impl Resolver { // succeeds or bails out (as importing * from an empty module or a module // that exports nothing is valid). fn resolve_glob_import(&mut self, - privacy: Privacy, - module_: @mut Module, - containing_module: @mut Module, - id: NodeId) - -> ResolveResult<()> { + module_: @mut Module, + containing_module: @mut Module, + id: NodeId, + is_public: bool, + lp: LastPrivate) + -> ResolveResult<()> { // This function works in a highly imperative manner; it eagerly adds // everything it can to the list of import resolutions of the module // node. - debug2!("(resolving glob import) resolving {:?} glob import", privacy); + debug2!("(resolving glob import) resolving glob import {}", id); // We must bail out if the node has unresolved imports of any kind // (including globs). @@ -2540,12 +2520,17 @@ impl Resolver { target_import_resolution.type_target.is_none(), self.module_to_str(module_)); + if !target_import_resolution.is_public { + debug2!("(resolving glob import) nevermind, just kidding"); + continue + } + // Here we merge two import resolutions. match module_.import_resolutions.find(ident) { - None if target_import_resolution.privacy == Public => { + None => { // Simple: just copy the old import resolution. let new_import_resolution = - @mut ImportResolution::new(privacy, id); + @mut ImportResolution::new(id, is_public); new_import_resolution.value_target = target_import_resolution.value_target; new_import_resolution.type_target = @@ -2554,7 +2539,6 @@ impl Resolver { module_.import_resolutions.insert (*ident, new_import_resolution); } - None => { /* continue ... */ } Some(&dest_import_resolution) => { // Merge the two import resolutions at a finer-grained // level. @@ -2577,6 +2561,7 @@ impl Resolver { Some(type_target); } } + dest_import_resolution.is_public = is_public; } } } @@ -2587,7 +2572,8 @@ impl Resolver { match module_.import_resolutions.find(&name) { None => { // Create a new import resolution from this child. - dest_import_resolution = @mut ImportResolution::new(privacy, id); + dest_import_resolution = @mut ImportResolution::new(id, + is_public); module_.import_resolutions.insert (name, dest_import_resolution); } @@ -2597,11 +2583,10 @@ impl Resolver { } debug2!("(resolving glob import) writing resolution `{}` in `{}` \ - to `{}`, privacy={:?}", + to `{}`", interner_get(name), self.module_to_str(containing_module), - self.module_to_str(module_), - dest_import_resolution.privacy); + self.module_to_str(module_)); // Merge the child item into the import resolution. if name_bindings.defined_in_public_namespace(ValueNS) { @@ -2616,6 +2601,7 @@ impl Resolver { Some(Target::new(containing_module, name_bindings)); dest_import_resolution.type_id = id; } + dest_import_resolution.is_public = is_public; }; // Add all children from the containing module. @@ -2635,6 +2621,7 @@ impl Resolver { match containing_module.def_id { Some(did) => { self.def_map.insert(id, DefMod(did)); + self.last_private.insert(id, lp); } None => {} } @@ -2645,15 +2632,17 @@ impl Resolver { /// Resolves the given module path from the given root `module_`. fn resolve_module_path_from_root(&mut self, - module_: @mut Module, - module_path: &[Ident], - index: uint, - span: Span, - mut name_search_type: NameSearchType) - -> ResolveResult<@mut Module> { + module_: @mut Module, + module_path: &[Ident], + index: uint, + span: Span, + name_search_type: NameSearchType, + lp: LastPrivate) + -> ResolveResult<(@mut Module, LastPrivate)> { let mut search_module = module_; let mut index = index; let module_path_len = module_path.len(); + let mut closest_private = lp; // Resolve the module part of the path. This does not involve looking // upward though scope chains; we simply resolve names directly in @@ -2690,7 +2679,7 @@ impl Resolver { self.session.str_of(name)); return Indeterminate; } - Success(target) => { + Success((target, used_proxy)) => { // Check to see whether there are type bindings, and, if // so, whether there is a module within. match target.bindings.type_def { @@ -2720,7 +2709,23 @@ impl Resolver { or type implementation"); return Failed; } - (_, _) => search_module = module_def, + (_, _) => { + search_module = module_def; + + // Keep track of the closest + // private module used when + // resolving this import chain. + if !used_proxy && + !search_module.is_public { + match search_module.def_id { + Some(did) => { + closest_private = + DependsOn(did); + } + None => {} + } + } + } } } } @@ -2738,28 +2743,23 @@ impl Resolver { } index += 1; - - // After the first element of the path, allow searching only - // through public identifiers. - // - // XXX: Rip this out and move it to the privacy checker. - if name_search_type == PathPublicOrPrivateSearch { - name_search_type = PathPublicOnlySearch - } } - return Success(search_module); + return Success((search_module, closest_private)); } /// Attempts to resolve the module part of an import directive or path /// rooted at the given module. + /// + /// On success, returns the resolved module, and the closest *private* + /// module found to the destination when resolving this path. fn resolve_module_path(&mut self, - module_: @mut Module, - module_path: &[Ident], - use_lexical_scope: UseLexicalScopeFlag, - span: Span, - name_search_type: NameSearchType) - -> ResolveResult<@mut Module> { + module_: @mut Module, + module_path: &[Ident], + use_lexical_scope: UseLexicalScopeFlag, + span: Span, + name_search_type: NameSearchType) + -> ResolveResult<(@mut Module, LastPrivate)> { let module_path_len = module_path.len(); assert!(module_path_len > 0); @@ -2774,6 +2774,7 @@ impl Resolver { let search_module; let start_index; + let last_private; match module_prefix_result { Failed => { let mpath = self.idents_to_str(module_path); @@ -2805,6 +2806,7 @@ impl Resolver { // resolution process at index zero. search_module = self.graph_root.get_module(); start_index = 0; + last_private = AllPublic; } UseLexicalScope => { // This is not a crate-relative path. We resolve the @@ -2826,6 +2828,7 @@ impl Resolver { Success(containing_module) => { search_module = containing_module; start_index = 1; + last_private = AllPublic; } } } @@ -2834,6 +2837,7 @@ impl Resolver { Success(PrefixFound(containing_module, index)) => { search_module = containing_module; start_index = index; + last_private = DependsOn(containing_module.def_id.unwrap()); } } @@ -2841,18 +2845,19 @@ impl Resolver { module_path, start_index, span, - name_search_type) + name_search_type, + last_private) } /// Invariant: This must only be called during main resolution, not during /// import resolution. fn resolve_item_in_lexical_scope(&mut self, - module_: @mut Module, - name: Ident, - namespace: Namespace, - search_through_modules: - SearchThroughModulesFlag) - -> ResolveResult { + module_: @mut Module, + name: Ident, + namespace: Namespace, + search_through_modules: + SearchThroughModulesFlag) + -> ResolveResult<(Target, bool)> { debug2!("(resolving item in lexical scope) resolving `{}` in \ namespace {:?} in `{}`", self.session.str_of(name), @@ -2865,7 +2870,8 @@ impl Resolver { match module_.children.find(&name.name) { Some(name_bindings) if name_bindings.defined_in_namespace(namespace) => { - return Success(Target::new(module_, *name_bindings)); + debug2!("top name bindings succeeded"); + return Success((Target::new(module_, *name_bindings), false)); } Some(_) | None => { /* Not found; continue. */ } } @@ -2890,7 +2896,7 @@ impl Resolver { debug2!("(resolving item in lexical scope) using \ import resolution"); self.used_imports.insert(import_resolution.id(namespace)); - return Success(target); + return Success((target, false)); } } } @@ -2904,7 +2910,8 @@ impl Resolver { let name_bindings = @mut Resolver::create_name_bindings_from_module( *module); - return Success(Target::new(module_, name_bindings)); + debug2!("lower name bindings succeeded"); + return Success((Target::new(module_, name_bindings), false)); } } } @@ -2954,7 +2961,7 @@ impl Resolver { match self.resolve_name_in_module(search_module, name, namespace, - PathPublicOrPrivateSearch) { + PathSearch) { Failed => { // Continue up the search chain. } @@ -2966,9 +2973,11 @@ impl Resolver { higher scope; bailing"); return Indeterminate; } - Success(target) => { + Success((target, used_reexport)) => { // We found the module. - return Success(target); + debug2!("(resolving item in lexical scope) found name \ + in module, done"); + return Success((target, used_reexport)); } } } @@ -2976,15 +2985,15 @@ impl Resolver { /// Resolves a module name in the current lexical scope. fn resolve_module_in_lexical_scope(&mut self, - module_: @mut Module, - name: Ident) - -> ResolveResult<@mut Module> { + module_: @mut Module, + name: Ident) + -> ResolveResult<@mut Module> { // If this module is an anonymous module, resolve the item in the // lexical scope. Otherwise, resolve the item from the crate root. let resolve_result = self.resolve_item_in_lexical_scope( module_, name, TypeNS, DontSearchThroughModules); match resolve_result { - Success(target) => { + Success((target, _)) => { let bindings = &mut *target.bindings; match bindings.type_def { Some(ref type_def) => { @@ -3064,8 +3073,8 @@ impl Resolver { /// (b) some chain of `super::`. /// grammar: (SELF MOD_SEP ) ? (SUPER MOD_SEP) * fn resolve_module_prefix(&mut self, - module_: @mut Module, - module_path: &[Ident]) + module_: @mut Module, + module_path: &[Ident]) -> ResolveResult { // Start at the current module if we see `self` or `super`, or at the // top of the crate otherwise. @@ -3106,12 +3115,15 @@ impl Resolver { /// Attempts to resolve the supplied name in the given module for the /// given namespace. If successful, returns the target corresponding to /// the name. + /// + /// The boolean returned on success is an indicator of whether this lookup + /// passed through a public re-export proxy. fn resolve_name_in_module(&mut self, - module_: @mut Module, - name: Ident, - namespace: Namespace, - name_search_type: NameSearchType) - -> ResolveResult { + module_: @mut Module, + name: Ident, + namespace: Namespace, + name_search_type: NameSearchType) + -> ResolveResult<(Target, bool)> { debug2!("(resolving name in module) resolving `{}` in `{}`", self.session.str_of(name), self.module_to_str(module_)); @@ -3122,7 +3134,7 @@ impl Resolver { Some(name_bindings) if name_bindings.defined_in_namespace(namespace) => { debug2!("(resolving name in module) found node as child"); - return Success(Target::new(module_, *name_bindings)); + return Success((Target::new(module_, *name_bindings), false)); } Some(_) | None => { // Continue. @@ -3133,39 +3145,30 @@ impl Resolver { // If this is a search of all imports, we should be done with glob // resolution at this point. - if name_search_type == PathPublicOrPrivateSearch || - name_search_type == PathPublicOnlySearch { + if name_search_type == PathSearch { assert_eq!(module_.glob_count, 0); } // Check the list of resolved imports. match module_.import_resolutions.find(&name.name) { Some(import_resolution) => { - if import_resolution.privacy == Public && + if import_resolution.is_public && import_resolution.outstanding_references != 0 { debug2!("(resolving name in module) import \ - unresolved; bailing out"); + unresolved; bailing out"); return Indeterminate; } - match import_resolution.target_for_namespace(namespace) { None => { debug2!("(resolving name in module) name found, \ but not in namespace {:?}", namespace); } - Some(target) - if name_search_type == - PathPublicOrPrivateSearch || - import_resolution.privacy == Public => { + Some(target) => { debug2!("(resolving name in module) resolved to \ import"); self.used_imports.insert(import_resolution.id(namespace)); - return Success(target); - } - Some(_) => { - debug2!("(resolving name in module) name found, \ - but not public"); + return Success((target, true)); } } } @@ -3180,7 +3183,7 @@ impl Resolver { let name_bindings = @mut Resolver::create_name_bindings_from_module( *module); - return Success(Target::new(module_, name_bindings)); + return Success((Target::new(module_, name_bindings), false)); } } } @@ -3299,14 +3302,13 @@ impl Resolver { } fn add_exports_of_namebindings(&mut self, - exports2: &mut ~[Export2], - name: Name, - namebindings: @mut NameBindings, - ns: Namespace, - reexport: bool) { - match (namebindings.def_for_namespace(ns), - namebindings.privacy_for_namespace(ns)) { - (Some(d), Some(Public)) => { + exports2: &mut ~[Export2], + name: Name, + namebindings: @mut NameBindings, + ns: Namespace, + reexport: bool) { + match namebindings.def_for_namespace(ns) { + Some(d) => { debug2!("(computing exports) YES: {} '{}' => {:?}", if reexport { ~"reexport" } else { ~"export"}, interner_get(name), @@ -3317,34 +3319,27 @@ impl Resolver { def_id: def_id_of_def(d) }); } - (Some(_), Some(privacy)) => { - debug2!("(computing reexports) NO: privacy {:?}", privacy); - } - (d_opt, p_opt) => { - debug2!("(computing reexports) NO: {:?}, {:?}", d_opt, p_opt); + d_opt => { + debug2!("(computing reexports) NO: {:?}", d_opt); } } } fn add_exports_for_module(&mut self, - exports2: &mut ~[Export2], - module_: @mut Module) { + exports2: &mut ~[Export2], + module_: @mut Module) { for (name, importresolution) in module_.import_resolutions.iter() { - if importresolution.privacy != Public { - debug2!("(computing exports) not reexporting private `{}`", - interner_get(*name)); - continue; - } + if !importresolution.is_public { continue } let xs = [TypeNS, ValueNS]; - for ns in xs.iter() { - match importresolution.target_for_namespace(*ns) { + for &ns in xs.iter() { + match importresolution.target_for_namespace(ns) { Some(target) => { debug2!("(computing exports) maybe reexport '{}'", interner_get(*name)); - self.add_exports_of_namebindings(&mut *exports2, + self.add_exports_of_namebindings(exports2, *name, target.bindings, - *ns, + ns, true) } _ => () @@ -3753,7 +3748,7 @@ impl Resolver { // Associate this type parameter with // the item that bound it self.record_def(type_parameter.id, - DefTyParamBinder(node_id)); + (DefTyParamBinder(node_id), AllPublic)); // plain insert (no renaming) function_type_rib.bindings.insert(ident.name, def_like); } @@ -4192,7 +4187,7 @@ impl Resolver { Some(&primitive_type) => { result_def = - Some(DefPrimTy(primitive_type)); + Some((DefPrimTy(primitive_type), AllPublic)); if path.segments .iter() @@ -4218,10 +4213,7 @@ impl Resolver { match result_def { None => { - match self.resolve_path(ty.id, - path, - TypeNS, - true) { + match self.resolve_path(ty.id, path, TypeNS, true) { Some(def) => { debug2!("(resolving type) resolved `{}` to \ type {:?}", @@ -4279,12 +4271,12 @@ impl Resolver { } fn resolve_pattern(&mut self, - pattern: @Pat, - mode: PatternBindingMode, - mutability: Mutability, - // Maps idents to the node ID for the (outermost) - // pattern that binds them - bindings_list: Option<@mut HashMap>) { + pattern: @Pat, + mode: PatternBindingMode, + mutability: Mutability, + // Maps idents to the node ID for the (outermost) + // pattern that binds them + bindings_list: Option<@mut HashMap>) { let pat_id = pattern.id; do walk_pat(pattern) |pattern| { match pattern.node { @@ -4304,7 +4296,7 @@ impl Resolver { let renamed = mtwt_resolve(ident); match self.resolve_bare_identifier_pattern(ident) { - FoundStructOrEnumVariant(def) + FoundStructOrEnumVariant(def, lp) if mode == RefutableMode => { debug2!("(resolving pattern) resolving `{}` to \ struct or enum variant", @@ -4314,9 +4306,9 @@ impl Resolver { pattern, binding_mode, "an enum variant"); - self.record_def(pattern.id, def); + self.record_def(pattern.id, (def, lp)); } - FoundStructOrEnumVariant(_) => { + FoundStructOrEnumVariant(*) => { self.resolve_error(pattern.span, format!("declaration of `{}` \ shadows an enum \ @@ -4324,7 +4316,7 @@ impl Resolver { struct in scope", interner_get(renamed))); } - FoundConst(def) if mode == RefutableMode => { + FoundConst(def, lp) if mode == RefutableMode => { debug2!("(resolving pattern) resolving `{}` to \ constant", interner_get(renamed)); @@ -4333,9 +4325,9 @@ impl Resolver { pattern, binding_mode, "a constant"); - self.record_def(pattern.id, def); + self.record_def(pattern.id, (def, lp)); } - FoundConst(_) => { + FoundConst(*) => { self.resolve_error(pattern.span, "only irrefutable patterns \ allowed here"); @@ -4367,7 +4359,7 @@ impl Resolver { // will be able to distinguish variants from // locals in patterns. - self.record_def(pattern.id, def); + self.record_def(pattern.id, (def, AllPublic)); // Add the binding to the local ribs, if it // doesn't already exist in the bindings list. (We @@ -4420,11 +4412,11 @@ impl Resolver { PatIdent(binding_mode, ref path, _) => { // This must be an enum variant, struct, or constant. match self.resolve_path(pat_id, path, ValueNS, false) { - Some(def @ DefVariant(*)) | - Some(def @ DefStruct(*)) => { + Some(def @ (DefVariant(*), _)) | + Some(def @ (DefStruct(*), _)) => { self.record_def(pattern.id, def); } - Some(def @ DefStatic(*)) => { + Some(def @ (DefStatic(*), _)) => { self.enforce_default_binding_mode( pattern, binding_mode, @@ -4455,10 +4447,10 @@ impl Resolver { PatEnum(ref path, _) => { // This must be an enum variant, struct or const. match self.resolve_path(pat_id, path, ValueNS, false) { - Some(def @ DefFn(*)) | - Some(def @ DefVariant(*)) | - Some(def @ DefStruct(*)) | - Some(def @ DefStatic(*)) => { + Some(def @ (DefFn(*), _)) | + Some(def @ (DefVariant(*), _)) | + Some(def @ (DefStruct(*), _)) | + Some(def @ (DefStatic(*), _)) => { self.record_def(pattern.id, def); } Some(_) => { @@ -4500,16 +4492,16 @@ impl Resolver { PatStruct(ref path, _, _) => { match self.resolve_path(pat_id, path, TypeNS, false) { - Some(DefTy(class_id)) + Some((DefTy(class_id), lp)) if self.structs.contains(&class_id) => { let class_def = DefStruct(class_id); - self.record_def(pattern.id, class_def); + self.record_def(pattern.id, (class_def, lp)); } - Some(definition @ DefStruct(class_id)) => { + Some(definition @ (DefStruct(class_id), _)) => { assert!(self.structs.contains(&class_id)); self.record_def(pattern.id, definition); } - Some(definition @ DefVariant(_, variant_id, _)) + Some(definition @ (DefVariant(_, variant_id, _), _)) if self.structs.contains(&variant_id) => { self.record_def(pattern.id, definition); } @@ -4538,19 +4530,25 @@ impl Resolver { name, ValueNS, SearchThroughModules) { - Success(target) => { + Success((target, _)) => { + debug2!("(resolve bare identifier pattern) succeeded in \ + finding {} at {:?}", + self.session.str_of(name), target.bindings.value_def); match target.bindings.value_def { None => { fail2!("resolved name in the value namespace to a \ set of name bindings with no def?!"); } Some(def) => { + // For the two success cases, this lookup can be + // considered as not having a private component because + // the lookup happened only within the current module. match def.def { def @ DefVariant(*) | def @ DefStruct(*) => { - return FoundStructOrEnumVariant(def); + return FoundStructOrEnumVariant(def, AllPublic); } def @ DefStatic(_, false) => { - return FoundConst(def); + return FoundConst(def, AllPublic); } _ => { return BareIdentifierPatternUnresolved; @@ -4565,6 +4563,8 @@ impl Resolver { } Failed => { + debug2!("(resolve bare identifier pattern) failed to find {}", + self.session.str_of(name)); return BareIdentifierPatternUnresolved; } } @@ -4573,35 +4573,31 @@ impl Resolver { /// If `check_ribs` is true, checks the local definitions first; i.e. /// doesn't skip straight to the containing module. fn resolve_path(&mut self, - id: NodeId, - path: &Path, - namespace: Namespace, - check_ribs: bool) - -> Option { + id: NodeId, + path: &Path, + namespace: Namespace, + check_ribs: bool) -> Option<(Def, LastPrivate)> { // First, resolve the types. for ty in path.segments.iter().flat_map(|s| s.types.iter()) { self.resolve_type(ty); } if path.global { - return self.resolve_crate_relative_path(path, - self.xray_context, - namespace); + return self.resolve_crate_relative_path(path, namespace); } - let unqualified_def = self.resolve_identifier(path.segments - .last() - .identifier, - namespace, - check_ribs, - path.span); + let unqualified_def = + self.resolve_identifier(path.segments + .last() + .identifier, + namespace, + check_ribs, + path.span); if path.segments.len() > 1 { - let def = self.resolve_module_relative_path(path, - self.xray_context, - namespace); + let def = self.resolve_module_relative_path(path, namespace); match (def, unqualified_def) { - (Some(d), Some(ud)) if d == ud => { + (Some((d, _)), Some((ud, _))) if d == ud => { self.session.add_lint(unnecessary_qualification, id, path.span, @@ -4622,13 +4618,13 @@ impl Resolver { namespace: Namespace, check_ribs: bool, span: Span) - -> Option { + -> Option<(Def, LastPrivate)> { if check_ribs { match self.resolve_identifier_in_local_ribs(identifier, namespace, span) { Some(def) => { - return Some(def); + return Some((def, AllPublic)); } None => { // Continue. @@ -4642,51 +4638,43 @@ impl Resolver { // FIXME #4952: Merge me with resolve_name_in_module? fn resolve_definition_of_name_in_module(&mut self, - containing_module: @mut Module, - name: Ident, - namespace: Namespace, - xray: XrayFlag) + containing_module: @mut Module, + name: Ident, + namespace: Namespace) -> NameDefinition { // First, search children. self.populate_module_if_necessary(containing_module); match containing_module.children.find(&name.name) { Some(child_name_bindings) => { - match (child_name_bindings.def_for_namespace(namespace), - child_name_bindings.privacy_for_namespace(namespace)) { - (Some(def), Some(Public)) => { - // Found it. Stop the search here. - return ChildNameDefinition(def); - } - (Some(def), _) if xray == Xray => { + match child_name_bindings.def_for_namespace(namespace) { + Some(def) => { // Found it. Stop the search here. - return ChildNameDefinition(def); - } - (Some(_), _) | (None, _) => { - // Continue. + let p = child_name_bindings.defined_in_public_namespace( + namespace); + let lp = if p {AllPublic} else { + DependsOn(def_id_of_def(def)) + }; + return ChildNameDefinition(def, lp); } + None => {} } } - None => { - // Continue. - } + None => {} } // Next, search import resolutions. match containing_module.import_resolutions.find(&name.name) { - Some(import_resolution) if import_resolution.privacy == Public || - xray == Xray => { + Some(import_resolution) if import_resolution.is_public => { match (*import_resolution).target_for_namespace(namespace) { Some(target) => { - match (target.bindings.def_for_namespace(namespace), - target.bindings.privacy_for_namespace( - namespace)) { - (Some(def), Some(Public)) => { + match target.bindings.def_for_namespace(namespace) { + Some(def) => { // Found it. let id = import_resolution.id(namespace); self.used_imports.insert(id); - return ImportNameDefinition(def); + return ImportNameDefinition(def, AllPublic); } - (Some(_), _) | (None, _) => { + None => { // This can happen with external impls, due to // the imperfect way we read the metadata. } @@ -4695,7 +4683,7 @@ impl Resolver { None => {} } } - Some(_) | None => {} // Continue. + Some(*) | None => {} // Continue. } // Finally, search through external children. @@ -4706,7 +4694,10 @@ impl Resolver { match module.def_id { None => {} // Continue. Some(def_id) => { - return ChildNameDefinition(DefMod(def_id)); + let lp = if module.is_public {AllPublic} else { + DependsOn(def_id) + }; + return ChildNameDefinition(DefMod(def_id), lp); } } } @@ -4719,17 +4710,17 @@ impl Resolver { // resolve a "module-relative" path, e.g. a::b::c fn resolve_module_relative_path(&mut self, path: &Path, - xray: XrayFlag, namespace: Namespace) - -> Option { + -> Option<(Def, LastPrivate)> { let module_path_idents = path.segments.init().map(|ps| ps.identifier); let containing_module; + let last_private; match self.resolve_module_path(self.current_module, module_path_idents, UseLexicalScope, path.span, - PathPublicOnlySearch) { + PathSearch) { Failed => { let msg = format!("use of undeclared module `{}`", self.idents_to_str(module_path_idents)); @@ -4741,22 +4732,22 @@ impl Resolver { fail2!("indeterminate unexpected"); } - Success(resulting_module) => { + Success((resulting_module, resulting_last_private)) => { containing_module = resulting_module; + last_private = resulting_last_private; } } let ident = path.segments.last().identifier; let def = match self.resolve_definition_of_name_in_module(containing_module, ident, - namespace, - xray) { + namespace) { NoNameDefinition => { // We failed to resolve the name. Report an error. return None; } - ChildNameDefinition(def) | ImportNameDefinition(def) => { - def + ChildNameDefinition(def, lp) | ImportNameDefinition(def, lp) => { + (def, last_private.or(lp)) } }; match containing_module.kind { @@ -4783,20 +4774,21 @@ impl Resolver { /// Invariant: This must be called only during main resolution, not during /// import resolution. fn resolve_crate_relative_path(&mut self, - path: &Path, - xray: XrayFlag, - namespace: Namespace) - -> Option { + path: &Path, + namespace: Namespace) + -> Option<(Def, LastPrivate)> { let module_path_idents = path.segments.init().map(|ps| ps.identifier); let root_module = self.graph_root.get_module(); let containing_module; + let last_private; match self.resolve_module_path_from_root(root_module, module_path_idents, 0, path.span, - PathPublicOrPrivateSearch) { + PathSearch, + AllPublic) { Failed => { let msg = format!("use of undeclared module `::{}`", self.idents_to_str(module_path_idents)); @@ -4808,22 +4800,22 @@ impl Resolver { fail2!("indeterminate unexpected"); } - Success(resulting_module) => { + Success((resulting_module, resulting_last_private)) => { containing_module = resulting_module; + last_private = resulting_last_private; } } let name = path.segments.last().identifier; match self.resolve_definition_of_name_in_module(containing_module, name, - namespace, - xray) { + namespace) { NoNameDefinition => { // We failed to resolve the name. Report an error. return None; } - ChildNameDefinition(def) | ImportNameDefinition(def) => { - return Some(def); + ChildNameDefinition(def, lp) | ImportNameDefinition(def, lp) => { + return Some((def, last_private.or(lp))); } } } @@ -4896,26 +4888,32 @@ impl Resolver { } fn resolve_item_by_identifier_in_lexical_scope(&mut self, - ident: Ident, - namespace: Namespace) - -> Option { + ident: Ident, + namespace: Namespace) + -> Option<(Def, LastPrivate)> { // Check the items. match self.resolve_item_in_lexical_scope(self.current_module, ident, namespace, DontSearchThroughModules) { - Success(target) => { + Success((target, _)) => { match (*target.bindings).def_for_namespace(namespace) { None => { // This can happen if we were looking for a type and // found a module instead. Modules don't have defs. + debug2!("(resolving item path by identifier in lexical \ + scope) failed to resolve {} after success...", + self.session.str_of(ident)); return None; } Some(def) => { debug2!("(resolving item path in lexical scope) \ resolved `{}` to item", self.session.str_of(ident)); - return Some(def); + // This lookup is "all public" because it only searched + // for one identifier in the current module (couldn't + // have passed through reexports or anything like that. + return Some((def, AllPublic)); } } } @@ -4923,6 +4921,8 @@ impl Resolver { fail2!("unexpected indeterminate result"); } Failed => { + debug2!("(resolving item path by identifier in lexical scope) \ + failed to resolve {}", self.session.str_of(ident)); return None; } } @@ -5005,7 +5005,7 @@ impl Resolver { // First-class methods are not supported yet; error // out here. match def { - DefMethod(*) => { + (DefMethod(*), _) => { self.resolve_error(expr.span, "first-class methods \ are not supported"); @@ -5027,7 +5027,7 @@ impl Resolver { // structs, which wouldn't result in this error.) match self.with_no_errors(|this| this.resolve_path(expr.id, path, TypeNS, false)) { - Some(DefTy(struct_id)) + Some((DefTy(struct_id), _)) if self.structs.contains(&struct_id) => { self.resolve_error(expr.span, format!("`{}` is a structure name, but \ @@ -5075,12 +5075,12 @@ impl Resolver { ExprStruct(ref path, _, _) => { // Resolve the path to the structure it goes to. match self.resolve_path(expr.id, path, TypeNS, false) { - Some(DefTy(class_id)) | Some(DefStruct(class_id)) + Some((DefTy(class_id), lp)) | Some((DefStruct(class_id), lp)) if self.structs.contains(&class_id) => { let class_def = DefStruct(class_id); - self.record_def(expr.id, class_def); + self.record_def(expr.id, (class_def, lp)); } - Some(definition @ DefVariant(_, class_id, _)) + Some(definition @ (DefVariant(_, class_id, _), _)) if self.structs.contains(&class_id) => { self.record_def(expr.id, definition); } @@ -5118,7 +5118,8 @@ impl Resolver { `{}`", interner_get(label))), Some(DlDef(def @ DefLabel(_))) => { - self.record_def(expr.id, def) + // XXX: is AllPublic correct? + self.record_def(expr.id, (def, AllPublic)) } Some(_) => { self.session.span_bug(expr.span, @@ -5135,7 +5136,7 @@ impl Resolver { "`self` is not allowed in \ this context") } - Some(def) => self.record_def(expr.id, def), + Some(def) => self.record_def(expr.id, (def, AllPublic)), } } @@ -5352,8 +5353,10 @@ impl Resolver { } } - fn record_def(&mut self, node_id: NodeId, def: Def) { - debug2!("(recording def) recording {:?} for {:?}", def, node_id); + fn record_def(&mut self, node_id: NodeId, (def, lp): (Def, LastPrivate)) { + debug2!("(recording def) recording {:?} for {:?}, last private {:?}", + def, node_id, lp); + self.last_private.insert(node_id, lp); do self.def_map.insert_or_update_with(node_id, def) |_, old_value| { // Resolve appears to "resolve" the same ID multiple // times, so here is a sanity check it at least comes to @@ -5393,7 +5396,7 @@ impl Resolver { } fn check_for_item_unused_imports(&self, vi: &view_item) { - // Ignore public import statements because there's no way to be sure + // Ignore is_public import statements because there's no way to be sure // whether they're used or not. Also ignore imports with a dummy span // because this means that they were generated in some fashion by the // compiler and we don't need to consider them. @@ -5499,7 +5502,9 @@ impl Resolver { pub struct CrateMap { def_map: DefMap, exp_map2: ExportMap2, - trait_map: TraitMap + trait_map: TraitMap, + external_exports: ExternalExports, + last_private_map: LastPrivateMap, } /// Entry point to crate resolution. @@ -5509,9 +5514,13 @@ pub fn resolve_crate(session: Session, -> CrateMap { let mut resolver = Resolver(session, lang_items, crate.span); resolver.resolve(crate); + let Resolver { def_map, export_map2, trait_map, last_private, + external_exports, _ } = resolver; CrateMap { - def_map: resolver.def_map, - exp_map2: resolver.export_map2, - trait_map: resolver.trait_map.clone(), + def_map: def_map, + exp_map2: export_map2, + trait_map: trait_map, + external_exports: external_exports, + last_private_map: last_private, } } diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 16ac802ee1571..73b197ec36199 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -3034,7 +3034,6 @@ pub fn crate_ctxt_to_encode_parms<'r>(cx: &'r CrateContext, ie: encoder::encode_ diag: diag, tcx: cx.tcx, reexports2: cx.exp_map2, - exported_items: cx.exported_items, item_symbols: item_symbols, discrim_symbols: discrim_symbols, non_inlineable_statics: &cx.non_inlineable_statics, @@ -3116,7 +3115,6 @@ pub fn trans_crate(sess: session::Session, llmod_id, analysis.ty_cx, analysis.exp_map2, - analysis.exported_items, analysis.maps, symbol_hasher, link_meta, diff --git a/src/librustc/middle/trans/context.rs b/src/librustc/middle/trans/context.rs index 3125155d0c793..2b3874e0bf0d8 100644 --- a/src/librustc/middle/trans/context.rs +++ b/src/librustc/middle/trans/context.rs @@ -49,7 +49,6 @@ pub struct CrateContext { intrinsics: HashMap<&'static str, ValueRef>, item_vals: HashMap, exp_map2: resolve::ExportMap2, - exported_items: @privacy::ExportedItems, reachable: @mut HashSet, item_symbols: HashMap, link_meta: LinkMeta, @@ -125,7 +124,6 @@ impl CrateContext { name: &str, tcx: ty::ctxt, emap2: resolve::ExportMap2, - exported_items: @privacy::ExportedItems, maps: astencode::Maps, symbol_hasher: hash::State, link_meta: LinkMeta, @@ -185,7 +183,6 @@ impl CrateContext { intrinsics: intrinsics, item_vals: HashMap::new(), exp_map2: emap2, - exported_items: exported_items, reachable: reachable, item_symbols: HashMap::new(), link_meta: link_meta, diff --git a/src/libsyntax/ast_util.rs b/src/libsyntax/ast_util.rs index 2c01f246c5f3d..926d1997465ab 100644 --- a/src/libsyntax/ast_util.rs +++ b/src/libsyntax/ast_util.rs @@ -702,32 +702,6 @@ pub fn struct_def_is_tuple_like(struct_def: &ast::struct_def) -> bool { struct_def.ctor_id.is_some() } -pub fn visibility_to_privacy(visibility: visibility) -> Privacy { - match visibility { - public => Public, - inherited | private => Private - } -} - -pub fn variant_visibility_to_privacy(visibility: visibility, - enclosing_is_public: bool) - -> Privacy { - if enclosing_is_public { - match visibility { - public | inherited => Public, - private => Private - } - } else { - visibility_to_privacy(visibility) - } -} - -#[deriving(Eq)] -pub enum Privacy { - Private, - Public -} - /// Returns true if the given pattern consists solely of an identifier /// and false otherwise. pub fn pat_is_ident(pat: @ast::Pat) -> bool { diff --git a/src/test/compile-fail/glob-resolve1.rs b/src/test/compile-fail/glob-resolve1.rs new file mode 100644 index 0000000000000..a0004f98ecf27 --- /dev/null +++ b/src/test/compile-fail/glob-resolve1.rs @@ -0,0 +1,45 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Make sure that globs only bring in public things. + +use bar::*; + +mod bar { + use import = self::fpriv; + fn fpriv() {} + extern { + fn epriv(); + } + enum A { A1 } + pub enum B { B1 } + + struct C; + + type D = int; +} + +fn foo() {} + +fn main() { + fpriv(); //~ ERROR: unresolved + epriv(); //~ ERROR: unresolved + A1; //~ ERROR: unresolved + B1; + C; //~ ERROR: unresolved + import(); //~ ERROR: unresolved + + foo::(); //~ ERROR: undeclared + //~^ ERROR: undeclared + foo::(); //~ ERROR: undeclared + //~^ ERROR: undeclared + foo::(); //~ ERROR: undeclared + //~^ ERROR: undeclared +} diff --git a/src/test/compile-fail/privacy1.rs b/src/test/compile-fail/privacy1.rs new file mode 100644 index 0000000000000..2e2b53331cafa --- /dev/null +++ b/src/test/compile-fail/privacy1.rs @@ -0,0 +1,168 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[no_std]; // makes debugging this test *a lot* easier (during resolve) + +mod bar { + // shouln't bring in too much + pub use self::glob::*; + + // 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 { + pub fn foo() {} + fn bar() {} + + pub fn foo2(&self) {} + fn bar2(&self) {} + } + + pub enum Enum { + priv Priv, + Pub + } + + mod baz { + pub struct A; + impl A { + pub fn foo() {} + fn bar() {} + + pub fn foo2(&self) {} + fn bar2(&self) {} + } + + // both of these are re-exported by `bar`, but only one should be + // validly re-exported + pub fn foo() {} + fn bar() {} + } + + extern { + fn epriv(); + pub fn epub(); + } + + fn test() { + self::Priv; + self::Pub; + unsafe { + epriv(); + epub(); + } + self::baz::A; + self::baz::A::foo(); + self::baz::A::bar(); //~ ERROR: method `bar` is private + self::baz::A.foo2(); + self::baz::A.bar2(); //~ ERROR: method `bar2` is private + + // this used to cause an ICE in privacy traversal. + super::gpub(); + } + + mod glob { + pub fn gpub() {} + fn gpriv() {} + } + + mod private { + fn ppriv() {} + } +} + +pub fn gpub() {} + +fn lol() { + bar::A; + bar::A::foo(); + bar::A::bar(); //~ ERROR: method `bar` is private + bar::A.foo2(); + bar::A.bar2(); //~ ERROR: method `bar2` is private +} + +mod foo { + fn test() { + ::bar::A::foo(); + ::bar::A::bar(); //~ ERROR: method `bar` is private + ::bar::A.foo2(); + ::bar::A.bar2(); //~ ERROR: method `bar2` is private + ::bar::baz::A::foo(); //~ ERROR: method `foo` is private + ::bar::baz::A::bar(); //~ ERROR: method `bar` is private + ::bar::baz::A.foo2(); //~ ERROR: struct `A` is private + ::bar::baz::A.bar2(); //~ ERROR: struct `A` is private + //~^ ERROR: method `bar2` is private + ::lol(); + + ::bar::Priv; //~ ERROR: variant `Priv` is private + ::bar::Pub; + + unsafe { + ::bar::epriv(); //~ ERROR: function `epriv` is private + ::bar::epub(); + } + + ::bar::foo(); + ::bar::bar(); + + ::bar::gpub(); + + ::bar::baz::foo(); //~ ERROR: function `foo` is private + ::bar::baz::bar(); //~ ERROR: function `bar` is private + } + + fn test2() { + use bar::baz::{foo, bar}; + //~^ ERROR: function `foo` is private + //~^^ ERROR: function `bar` is private + foo(); + bar(); + } + + fn test3() { + use bar::baz; + //~^ ERROR: module `baz` is private + } + + fn test4() { + use bar::{foo, bar}; + foo(); + bar(); + } + + fn test5() { + use bar; + bar::foo(); + bar::bar(); + } +} + +pub mod mytest { + // Even though the inner `A` struct is a publicly exported item (usable from + // external crates through `foo::foo`, it should not be accessible through + // its definition path (which has the private `i` module). + use self::foo::foo; + use self::foo::i::A; //~ ERROR: type `A` is private + + pub mod foo { + pub use foo = self::i::A; + + mod i { + pub struct A; + } + } +} + +#[start] fn main(_: int, _: **u8) -> int { 3 } diff --git a/src/test/compile-fail/privacy2.rs b/src/test/compile-fail/privacy2.rs new file mode 100644 index 0000000000000..e8e21021cccd6 --- /dev/null +++ b/src/test/compile-fail/privacy2.rs @@ -0,0 +1,37 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[no_std]; // makes debugging this test *a lot* easier (during resolve) + +// Test to make sure that globs don't leak in regular `use` statements. + +mod bar { + pub use self::glob::*; + + mod glob { + use foo; + } +} + +pub fn foo() {} + +fn test1() { + use bar::foo; //~ ERROR: unresolved import + //~^ ERROR: failed to resolve +} + +fn test2() { + use bar::glob::foo; + //~^ ERROR: there is no + //~^^ ERROR: failed to resolve +} + +#[start] fn main(_: int, _: **u8) -> int { 3 } + diff --git a/src/test/compile-fail/privacy3.rs b/src/test/compile-fail/privacy3.rs new file mode 100644 index 0000000000000..523d4cd4b8ddd --- /dev/null +++ b/src/test/compile-fail/privacy3.rs @@ -0,0 +1,32 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[no_std]; // makes debugging this test *a lot* easier (during resolve) + +// Test to make sure that private items imported through globs remain private +// when they're used. + +mod bar { + pub use self::glob::*; + + mod glob { + fn gpriv() {} + } +} + +pub fn foo() {} + +fn test1() { + use bar::gpriv; //~ ERROR: unresolved import + //~^ ERROR: failed to resolve + gpriv(); +} + +#[start] fn main(_: int, _: **u8) -> int { 3 } diff --git a/src/test/compile-fail/privacy4.rs b/src/test/compile-fail/privacy4.rs new file mode 100644 index 0000000000000..88a9b2f30580b --- /dev/null +++ b/src/test/compile-fail/privacy4.rs @@ -0,0 +1,31 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#[no_std]; // makes debugging this test *a lot* easier (during resolve) + +// Test to make sure that private items imported through globs remain private +// when they're used. + +mod bar { + pub use self::glob::*; + + mod glob { + fn gpriv() {} + } +} + +pub fn foo() {} + +fn test2() { + use bar::glob::gpriv; //~ ERROR: function `gpriv` is private + gpriv(); +} + +#[start] fn main(_: int, _: **u8) -> int { 3 } From de7d1431760c788e5a471194fa85675033d0ed72 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 5 Oct 2013 14:44:37 -0700 Subject: [PATCH 2/4] Fix existing privacy/visibility violations This commit fixes all of the fallout of the previous commit which is an attempt to refine privacy. There were a few unfortunate leaks which now must be plugged, and the most horrible one is the current `shouldnt_be_public` module now inside `std::rt`. I think that this either needs a slight reorganization of the runtime, or otherwise it needs to just wait for the external users of these modules to get replaced with their `rt` implementations. Other fixes involve making things pub which should be pub, and otherwise updating error messages that now reference privacy instead of referencing an "unresolved name" (yay!). --- src/libextra/container.rs | 2 +- src/libextra/crypto/cryptoutil.rs | 2 +- src/libextra/stats.rs | 3 +- .../middle/borrowck/gather_loans/mod.rs | 4 +- src/librustc/middle/trans/datum.rs | 2 +- src/librustc/middle/trans/expr.rs | 6 +-- src/librustc/middle/trans/glue.rs | 4 +- src/librustc/middle/typeck/coherence.rs | 4 +- src/librustdoc/html/render.rs | 2 +- src/librusti/program.rs | 2 +- src/libstd/num/cmath.rs | 4 +- src/libstd/rt/io/buffered.rs | 5 ++- src/libstd/rt/io/mod.rs | 3 +- src/libstd/rt/mod.rs | 17 ++++++++- src/libstd/rt/sched.rs | 6 +++ src/libstd/select.rs | 4 +- src/libstd/task/mod.rs | 2 +- src/libstd/task/spawn.rs | 11 ++---- src/libstd/unstable/mod.rs | 2 +- src/libstd/util.rs | 2 +- src/libsyntax/ast_util.rs | 2 +- src/libsyntax/ext/expand.rs | 6 ++- src/test/compile-fail/export-import.rs | 3 +- src/test/compile-fail/export-tag-variant.rs | 4 +- src/test/compile-fail/issue-3993-2.rs | 5 +-- src/test/compile-fail/issue-3993-3.rs | 8 ++-- src/test/compile-fail/issue-3993.rs | 7 ++-- src/test/compile-fail/issue-4366-2.rs | 38 +++++++++++++++++++ src/test/compile-fail/issue-4366.rs | 7 +--- .../compile-fail/macro-local-data-key-priv.rs | 2 +- src/test/compile-fail/private-item-simple.rs | 2 +- src/test/compile-fail/private-variant.rs | 2 +- .../compile-fail/static-priv-by-default.rs | 6 +-- .../compile-fail/static-priv-by-default2.rs | 23 ++++++++--- src/test/compile-fail/xc-private-method.rs | 4 +- .../compile-fail/xcrate-private-by-default.rs | 23 +++++------ src/test/run-pass/export-non-interference2.rs | 2 +- src/test/run-pass/foreign-dupe.rs | 4 +- src/test/run-pass/foreign-no-abi.rs | 2 +- src/test/run-pass/intrinsic-uninit.rs | 2 +- src/test/run-pass/intrinsics-integer.rs | 36 +++++++++--------- .../mod_dir_simple/load_another_mod.rs | 2 +- src/test/run-pass/rec-align-u64.rs | 2 +- 43 files changed, 168 insertions(+), 111 deletions(-) create mode 100644 src/test/compile-fail/issue-4366-2.rs diff --git a/src/libextra/container.rs b/src/libextra/container.rs index c5311d210ab9b..cbc2f08679bb6 100644 --- a/src/libextra/container.rs +++ b/src/libextra/container.rs @@ -40,7 +40,7 @@ pub trait Deque : Mutable { } #[cfg(test)] -mod bench { +pub mod bench { use std::container::MutableMap; use std::{vec, rand}; use std::rand::Rng; diff --git a/src/libextra/crypto/cryptoutil.rs b/src/libextra/crypto/cryptoutil.rs index 8c97f7db2bd8a..f4bc87ae76392 100644 --- a/src/libextra/crypto/cryptoutil.rs +++ b/src/libextra/crypto/cryptoutil.rs @@ -346,7 +346,7 @@ impl StandardPadding for T { #[cfg(test)] -mod test { +pub mod test { use std::rand::{IsaacRng, Rng}; use std::vec; diff --git a/src/libextra/stats.rs b/src/libextra/stats.rs index 64c4a4a03fd56..9ac0d73c2ecf5 100644 --- a/src/libextra/stats.rs +++ b/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, diff --git a/src/librustc/middle/borrowck/gather_loans/mod.rs b/src/librustc/middle/borrowck/gather_loans/mod.rs index b3980c2e045b9..aaaa893c3e5c3 100644 --- a/src/librustc/middle/borrowck/gather_loans/mod.rs +++ b/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; diff --git a/src/librustc/middle/trans/datum.rs b/src/librustc/middle/trans/datum.rs index a70b38fca173c..3de3f99020e65 100644 --- a/src/librustc/middle/trans/datum.rs +++ b/src/librustc/middle/trans/datum.rs @@ -581,7 +581,7 @@ impl Datum { if !header && !ty::type_contents(bcx.tcx(), content_ty).contains_managed() { let ptr = self.to_value_llval(bcx); - let ty = type_of(bcx.ccx(), content_ty); + let ty = type_of::type_of(bcx.ccx(), content_ty); let body = PointerCast(bcx, ptr, ty.ptr_to()); Datum {val: body, ty: content_ty, mode: ByRef(ZeroMem)} } else { // has a header diff --git a/src/librustc/middle/trans/expr.rs b/src/librustc/middle/trans/expr.rs index 098f0e3db7c90..a6cf634499904 100644 --- a/src/librustc/middle/trans/expr.rs +++ b/src/librustc/middle/trans/expr.rs @@ -1026,7 +1026,7 @@ fn trans_lvalue_unadjusted(bcx: @mut Block, expr: &ast::Expr) -> DatumBlock { // which may not be equal to the enum's type for // non-C-like enums. let val = base::get_item_val(bcx.ccx(), did.node); - let pty = type_of(bcx.ccx(), const_ty).ptr_to(); + let pty = type_of::type_of(bcx.ccx(), const_ty).ptr_to(); PointerCast(bcx, val, pty) } else { { @@ -1040,7 +1040,7 @@ fn trans_lvalue_unadjusted(bcx: @mut Block, expr: &ast::Expr) -> DatumBlock { } unsafe { - let llty = type_of(bcx.ccx(), const_ty); + let llty = type_of::type_of(bcx.ccx(), const_ty); let symbol = csearch::get_symbol( bcx.ccx().sess.cstore, did); @@ -1396,7 +1396,7 @@ fn trans_unary_datum(bcx: @mut Block, heap: heap) -> DatumBlock { let _icx = push_ctxt("trans_boxed_expr"); if heap == heap_exchange { - let llty = type_of(bcx.ccx(), contents_ty); + let llty = type_of::type_of(bcx.ccx(), contents_ty); let size = llsize_of(bcx.ccx(), llty); let Result { bcx: bcx, val: val } = malloc_raw_dyn(bcx, contents_ty, heap_exchange, size); diff --git a/src/librustc/middle/trans/glue.rs b/src/librustc/middle/trans/glue.rs index d420514e84f7f..0f7e1db158328 100644 --- a/src/librustc/middle/trans/glue.rs +++ b/src/librustc/middle/trans/glue.rs @@ -203,7 +203,7 @@ pub fn lazily_emit_tydesc_glue(ccx: @mut CrateContext, field: uint, ti: @mut tydesc_info) { let _icx = push_ctxt("lazily_emit_tydesc_glue"); - let llfnty = Type::glue_fn(type_of::type_of(ccx, ti.ty).ptr_to()); + let llfnty = Type::glue_fn(type_of(ccx, ti.ty).ptr_to()); if lazily_emit_simplified_tydesc_glue(ccx, field, ti) { return; @@ -345,7 +345,7 @@ pub fn make_visit_glue(bcx: @mut Block, v: ValueRef, t: ty::t) -> @mut Block { bcx.tcx().sess.fatal(s); } }; - let v = PointerCast(bcx, v, type_of::type_of(bcx.ccx(), object_ty).ptr_to()); + let v = PointerCast(bcx, v, type_of(bcx.ccx(), object_ty).ptr_to()); bcx = reflect::emit_calls_to_trait_visit_ty(bcx, t, v, visitor_trait.def_id); // The visitor is a boxed object and needs to be dropped add_clean(bcx, v, object_ty); diff --git a/src/librustc/middle/typeck/coherence.rs b/src/librustc/middle/typeck/coherence.rs index 6adbcbf89d755..ae8081df7bac2 100644 --- a/src/librustc/middle/typeck/coherence.rs +++ b/src/librustc/middle/typeck/coherence.rs @@ -330,8 +330,8 @@ impl CoherenceChecker { let impl_poly_type = ty::lookup_item_type(tcx, impl_id); - let provided = ty::provided_trait_methods(tcx, trait_ref.def_id); - for trait_method in provided.iter() { + let prov = ty::provided_trait_methods(tcx, trait_ref.def_id); + for trait_method in prov.iter() { // Synthesize an ID. let new_id = tcx.sess.next_node_id(); let new_did = local_def(new_id); diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index f04527ee893b0..acb8720c786b7 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -126,7 +126,7 @@ enum Implementor { /// to be a fairly large and expensive structure to clone. Instead this adheres /// to both `Send` and `Freeze` so it may be stored in a `RWArc` instance and /// shared among the various rendering tasks. -struct Cache { +pub struct Cache { /// Mapping of typaram ids to the name of the type parameter. This is used /// when pretty-printing a type (so pretty printing doesn't have to /// painfully maintain a context like this) diff --git a/src/librusti/program.rs b/src/librusti/program.rs index 0053d7137768d..4ab9ac4aef5f3 100644 --- a/src/librusti/program.rs +++ b/src/librusti/program.rs @@ -24,7 +24,7 @@ use utils::*; /// This structure keeps track of the state of the world for the code being /// executed in rusti. #[deriving(Clone)] -struct Program { +pub struct Program { /// All known local variables local_vars: HashMap<~str, LocalVariable>, /// New variables which will be present (learned from typechecking) diff --git a/src/libstd/num/cmath.rs b/src/libstd/num/cmath.rs index 38923c5bda45d..0c515538266d5 100644 --- a/src/libstd/num/cmath.rs +++ b/src/libstd/num/cmath.rs @@ -36,10 +36,10 @@ pub mod c_double_utils { pub fn exp(n: c_double) -> c_double; // rename: for consistency with underscore usage elsewhere #[link_name="expm1"] - fn exp_m1(n: c_double) -> c_double; + pub fn exp_m1(n: c_double) -> c_double; pub fn exp2(n: c_double) -> c_double; #[link_name="fabs"] - fn abs(n: c_double) -> c_double; + pub fn abs(n: c_double) -> c_double; // rename: for clarity and consistency with add/sub/mul/div #[link_name="fdim"] pub fn abs_sub(a: c_double, b: c_double) -> c_double; diff --git a/src/libstd/rt/io/buffered.rs b/src/libstd/rt/io/buffered.rs index a8cf8151499f0..2269469ee23cc 100644 --- a/src/libstd/rt/io/buffered.rs +++ b/src/libstd/rt/io/buffered.rs @@ -335,14 +335,15 @@ mod test { // newtype struct autoderef weirdness #[test] fn test_buffered_stream() { + use rt; struct S; - impl Writer for S { + impl rt::io::Writer for S { fn write(&mut self, _: &[u8]) {} fn flush(&mut self) {} } - impl Reader for S { + impl rt::io::Reader for S { fn read(&mut self, _: &mut [u8]) -> Option { None } fn eof(&mut self) -> bool { true } } diff --git a/src/libstd/rt/io/mod.rs b/src/libstd/rt/io/mod.rs index c2f137ba119ab..a18f97930fa2f 100644 --- a/src/libstd/rt/io/mod.rs +++ b/src/libstd/rt/io/mod.rs @@ -300,7 +300,8 @@ pub mod comm_adapters; mod extensions; /// Non-I/O things needed by the I/O module -mod support; +// XXX: shouldn this really be pub? +pub mod support; /// Basic Timer pub mod timer; diff --git a/src/libstd/rt/mod.rs b/src/libstd/rt/mod.rs index 2ece2800cf267..c7c4d3fc4f625 100644 --- a/src/libstd/rt/mod.rs +++ b/src/libstd/rt/mod.rs @@ -67,14 +67,27 @@ use rt::local::Local; use rt::sched::{Scheduler, Shutdown}; use rt::sleeper_list::SleeperList; use rt::task::{Task, SchedTask, GreenTask, Sched}; -use rt::thread::Thread; -use rt::work_queue::WorkQueue; use rt::uv::uvio::UvEventLoop; use unstable::atomics::{AtomicInt, SeqCst}; use unstable::sync::UnsafeArc; use vec; use vec::{OwnedVector, MutableVector, ImmutableVector}; +use self::thread::Thread; +use self::work_queue::WorkQueue; + +// XXX: these probably shouldn't be public... +#[doc(hidden)] +pub mod shouldnt_be_public { + pub use super::sched::Scheduler; + pub use super::kill::KillHandle; + pub use super::thread::Thread; + pub use super::work_queue::WorkQueue; + pub use super::select::SelectInner; + pub use super::rtio::EventLoop; + pub use super::select::{SelectInner, SelectPortInner}; +} + /// The global (exchange) heap. pub mod global_heap; diff --git a/src/libstd/rt/sched.rs b/src/libstd/rt/sched.rs index bddcb700433c7..cbffec51cc969 100644 --- a/src/libstd/rt/sched.rs +++ b/src/libstd/rt/sched.rs @@ -803,6 +803,12 @@ impl SchedHandle { self.queue.push(msg); self.remote.fire(); } + pub fn send_task_from_friend(&mut self, friend: ~Task) { + self.send(TaskFromFriend(friend)); + } + pub fn send_shutdown(&mut self) { + self.send(Shutdown); + } } struct CleanupJob { diff --git a/src/libstd/select.rs b/src/libstd/select.rs index 2554a0ad58823..049b301144bed 100644 --- a/src/libstd/select.rs +++ b/src/libstd/select.rs @@ -15,10 +15,8 @@ use iter::{Iterator, DoubleEndedIterator}; use option::*; // use either::{Either, Left, Right}; // use rt::kill::BlockedTask; -use rt::sched::Scheduler; -use rt::select::{SelectInner, SelectPortInner}; use rt::local::Local; -use rt::rtio::EventLoop; +use rt::shouldnt_be_public::{EventLoop, Scheduler, SelectInner, SelectPortInner}; use task; use unstable::finally::Finally; use vec::{OwnedVector, MutableVector}; diff --git a/src/libstd/task/mod.rs b/src/libstd/task/mod.rs index 8e5353341eac8..cb45c6f78ebf5 100644 --- a/src/libstd/task/mod.rs +++ b/src/libstd/task/mod.rs @@ -551,7 +551,7 @@ pub fn deschedule() { //! Yield control to the task scheduler use rt::local::Local; - use rt::sched::Scheduler; + use rt::shouldnt_be_public::Scheduler; // FIXME(#7544): Optimize this, since we know we won't block. let sched: ~Scheduler = Local::take(); diff --git a/src/libstd/task/spawn.rs b/src/libstd/task/spawn.rs index a77c974429802..a801bf3328db2 100644 --- a/src/libstd/task/spawn.rs +++ b/src/libstd/task/spawn.rs @@ -89,11 +89,8 @@ use unstable::sync::Exclusive; use rt::in_green_task_context; use rt::local::Local; use rt::task::{Task, Sched}; -use rt::kill::KillHandle; -use rt::sched::Scheduler; +use rt::shouldnt_be_public::{Scheduler, KillHandle, WorkQueue, Thread}; use rt::uv::uvio::UvEventLoop; -use rt::thread::Thread; -use rt::work_queue::WorkQueue; #[cfg(test)] use task::default_task_opts; #[cfg(test)] use comm; @@ -556,8 +553,6 @@ fn enlist_many(child: &KillHandle, child_arc: &TaskGroupArc, } pub fn spawn_raw(mut opts: TaskOpts, f: ~fn()) { - use rt::sched::*; - rtassert!(in_green_task_context()); let child_data = Cell::new(gen_child_taskgroup(opts.linked, opts.supervised)); @@ -622,7 +617,7 @@ pub fn spawn_raw(mut opts: TaskOpts, f: ~fn()) { let mut new_sched_handle = new_sched.make_handle(); // Allow the scheduler to exit when the pinned task exits - new_sched_handle.send(Shutdown); + new_sched_handle.send_shutdown(); // Pin the new task to the new scheduler let new_task = if opts.watched { @@ -660,7 +655,7 @@ pub fn spawn_raw(mut opts: TaskOpts, f: ~fn()) { rtdebug!("enqueing join_task"); // Now tell the original scheduler to join with this thread // by scheduling a thread-joining task on the original scheduler - orig_sched_handle.send(TaskFromFriend(join_task)); + orig_sched_handle.send_task_from_friend(join_task); // NB: We can't simply send a message from here to another task // because this code isn't running in a task and message passing doesn't diff --git a/src/libstd/unstable/mod.rs b/src/libstd/unstable/mod.rs index e16e6384a4f16..0e281f33e2a3d 100644 --- a/src/libstd/unstable/mod.rs +++ b/src/libstd/unstable/mod.rs @@ -38,7 +38,7 @@ a normal large stack. */ pub fn run_in_bare_thread(f: ~fn()) { use cell::Cell; - use rt::thread::Thread; + use rt::shouldnt_be_public::Thread; let f_cell = Cell::new(f); let (port, chan) = comm::stream(); diff --git a/src/libstd/util.rs b/src/libstd/util.rs index 64bdc7fe8cd8b..44cfdb860576b 100644 --- a/src/libstd/util.rs +++ b/src/libstd/util.rs @@ -90,10 +90,10 @@ mod tests { use super::*; use clone::Clone; + use ops::Drop; use option::{None, Some}; use either::{Either, Left, Right}; use sys::size_of; - use kinds::Drop; #[test] fn identity_crisis() { diff --git a/src/libsyntax/ast_util.rs b/src/libsyntax/ast_util.rs index 926d1997465ab..d9e24e045ffd2 100644 --- a/src/libsyntax/ast_util.rs +++ b/src/libsyntax/ast_util.rs @@ -964,7 +964,7 @@ mod test { use super::*; use std::io; use opt_vec; - use std::hash::HashMap; + use std::hashmap::HashMap; fn ident_to_segment(id : &Ident) -> PathSegment { PathSegment{identifier:id.clone(), lifetime: None, types: opt_vec::Empty} diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 39540deb38f21..1039ec078049f 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -1551,7 +1551,8 @@ mod test { let varrefs = @mut ~[]; visit::walk_crate(&mut new_path_finder(varrefs), &renamed_ast, ()); match varrefs { - @[Path{segments:[ref seg],_}] => assert_eq!(mtwt_resolve(seg.identifier),a2_name), + @[ast::Path{segments:[ref seg],_}] => + assert_eq!(mtwt_resolve(seg.identifier),a2_name), _ => assert_eq!(0,1) } @@ -1565,7 +1566,8 @@ mod test { let varrefs = @mut ~[]; visit::walk_crate(&mut new_path_finder(varrefs), &double_renamed, ()); match varrefs { - @[Path{segments:[ref seg],_}] => assert_eq!(mtwt_resolve(seg.identifier),a3_name), + @[ast::Path{segments:[ref seg],_}] => + assert_eq!(mtwt_resolve(seg.identifier),a3_name), _ => assert_eq!(0,1) } } diff --git a/src/test/compile-fail/export-import.rs b/src/test/compile-fail/export-import.rs index a7578f6104f35..3877250126d34 100644 --- a/src/test/compile-fail/export-import.rs +++ b/src/test/compile-fail/export-import.rs @@ -8,9 +8,8 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// error-pattern: import - use m::unexported; +//~^ ERROR: is private mod m { pub fn exported() { } diff --git a/src/test/compile-fail/export-tag-variant.rs b/src/test/compile-fail/export-tag-variant.rs index 629699ca6a490..d92cd20485083 100644 --- a/src/test/compile-fail/export-tag-variant.rs +++ b/src/test/compile-fail/export-tag-variant.rs @@ -8,12 +8,10 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// error-pattern: unresolved name - mod foo { pub fn x() { } enum y { y1, } } -fn main() { let z = foo::y1; } +fn main() { let z = foo::y1; } //~ ERROR: is private diff --git a/src/test/compile-fail/issue-3993-2.rs b/src/test/compile-fail/issue-3993-2.rs index 2ca871cd11ca0..61980abdfe7af 100644 --- a/src/test/compile-fail/issue-3993-2.rs +++ b/src/test/compile-fail/issue-3993-2.rs @@ -8,12 +8,11 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use zoo::{duck, goose}; //~ ERROR failed to resolve import - //~^ ERROR unresolved import: found `goose` in `zoo` but it is private +use zoo::{duck, goose}; //~ ERROR: variant `goose` is private mod zoo { pub enum bird { - pub duck, + pub duck, //~ ERROR: unnecessary `pub` visibility priv goose } } diff --git a/src/test/compile-fail/issue-3993-3.rs b/src/test/compile-fail/issue-3993-3.rs index cab999f621de8..fae5eb5127237 100644 --- a/src/test/compile-fail/issue-3993-3.rs +++ b/src/test/compile-fail/issue-3993-3.rs @@ -8,13 +8,13 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use zoo::fly; //~ ERROR failed to resolve import - //~^ ERROR unresolved import: found `fly` in `zoo` but it is private +use zoo::fly; //~ ERROR: function `fly` is private mod zoo { - type fly = (); fn fly() {} } -fn main() {} +fn main() { + fly(); +} diff --git a/src/test/compile-fail/issue-3993.rs b/src/test/compile-fail/issue-3993.rs index 53a56ad277423..fae5eb5127237 100644 --- a/src/test/compile-fail/issue-3993.rs +++ b/src/test/compile-fail/issue-3993.rs @@ -8,12 +8,13 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use zoo::fly; //~ ERROR failed to resolve import - //~^ ERROR unresolved import: found `fly` in `zoo` but it is private +use zoo::fly; //~ ERROR: function `fly` is private mod zoo { fn fly() {} } -fn main() {} +fn main() { + fly(); +} diff --git a/src/test/compile-fail/issue-4366-2.rs b/src/test/compile-fail/issue-4366-2.rs new file mode 100644 index 0000000000000..4530267f35ff9 --- /dev/null +++ b/src/test/compile-fail/issue-4366-2.rs @@ -0,0 +1,38 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + + +// ensures that 'use foo:*' doesn't import non-public item + +use m1::*; + +mod foo { + pub fn foo() {} +} +mod a { + pub mod b { + use foo::foo; + type bar = int; + } + pub mod sub { + use a::b::*; + fn sub() -> bar { 1 } + //~^ ERROR: undeclared type name + } +} + +mod m1 { + fn foo() {} +} + +fn main() { + foo(); //~ ERROR: unresolved name +} + diff --git a/src/test/compile-fail/issue-4366.rs b/src/test/compile-fail/issue-4366.rs index 6b84d897c87bd..e9c1092a4a559 100644 --- a/src/test/compile-fail/issue-4366.rs +++ b/src/test/compile-fail/issue-4366.rs @@ -27,8 +27,7 @@ mod a { } pub mod sub { use a::b::*; - fn sub() -> bar { foo(); 1 } //~ ERROR: unresolved name `foo` - //~^ ERROR: use of undeclared type name `bar` + fn sub() -> int { foo(); 1 } //~ ERROR: unresolved name `foo` } } @@ -36,6 +35,4 @@ mod m1 { fn foo() {} } -fn main() { - foo(); //~ ERROR: unresolved name `foo` -} +fn main() {} diff --git a/src/test/compile-fail/macro-local-data-key-priv.rs b/src/test/compile-fail/macro-local-data-key-priv.rs index 463ad958f11b6..e87d57aaa5623 100644 --- a/src/test/compile-fail/macro-local-data-key-priv.rs +++ b/src/test/compile-fail/macro-local-data-key-priv.rs @@ -18,5 +18,5 @@ mod bar { fn main() { local_data::set(bar::baz, -10.0); - //~^ ERROR unresolved name `bar::baz` + //~^ ERROR static `baz` is private } diff --git a/src/test/compile-fail/private-item-simple.rs b/src/test/compile-fail/private-item-simple.rs index a31d0030f67e7..2b9e32b8f5860 100644 --- a/src/test/compile-fail/private-item-simple.rs +++ b/src/test/compile-fail/private-item-simple.rs @@ -13,5 +13,5 @@ mod a { } fn main() { - a::f(); //~ ERROR unresolved name + a::f(); //~ ERROR function `f` is private } diff --git a/src/test/compile-fail/private-variant.rs b/src/test/compile-fail/private-variant.rs index 3285997523a71..d63d04c90ccca 100644 --- a/src/test/compile-fail/private-variant.rs +++ b/src/test/compile-fail/private-variant.rs @@ -17,5 +17,5 @@ mod a { } fn main() { - let x = a::Liege; //~ ERROR unresolved name + let x = a::Liege; //~ ERROR variant `Liege` is private } diff --git a/src/test/compile-fail/static-priv-by-default.rs b/src/test/compile-fail/static-priv-by-default.rs index 59d7e23855c18..f447a6c547c7b 100644 --- a/src/test/compile-fail/static-priv-by-default.rs +++ b/src/test/compile-fail/static-priv-by-default.rs @@ -24,15 +24,15 @@ mod child { fn foo(_: int) {} fn full_ref() { - foo(static_priv_by_default::private); //~ ERROR: unresolved name + foo(static_priv_by_default::private); //~ ERROR: static `private` is private foo(static_priv_by_default::public); - foo(child::childs_child::private); //~ ERROR: unresolved name + foo(child::childs_child::private); //~ ERROR: static `private` is private foo(child::childs_child::public); } fn medium_ref() { use child::childs_child; - foo(childs_child::private); //~ ERROR: unresolved name + foo(childs_child::private); //~ ERROR: static `private` is private foo(childs_child::public); } diff --git a/src/test/compile-fail/static-priv-by-default2.rs b/src/test/compile-fail/static-priv-by-default2.rs index 28a17cf5e1c5c..423d182dd6920 100644 --- a/src/test/compile-fail/static-priv-by-default2.rs +++ b/src/test/compile-fail/static-priv-by-default2.rs @@ -19,11 +19,22 @@ mod child { } } -fn main() { - use static_priv_by_default::private; //~ ERROR: unresolved import - //~^ ERROR: failed to resolve - use static_priv_by_default::public; - use child::childs_child::private; //~ ERROR: unresolved import - //~^ ERROR: failed to resolve +fn foo(_: T) {} + +fn test1() { + use child::childs_child::private; + //~^ ERROR: static `private` is private use child::childs_child::public; + + foo(private); +} + +fn test2() { + use static_priv_by_default::private; + //~^ ERROR: static `private` is private + use static_priv_by_default::public; + + foo(private); } + +fn main() {} diff --git a/src/test/compile-fail/xc-private-method.rs b/src/test/compile-fail/xc-private-method.rs index b4a999766b541..a0a411ec9b03c 100644 --- a/src/test/compile-fail/xc-private-method.rs +++ b/src/test/compile-fail/xc-private-method.rs @@ -15,8 +15,8 @@ extern mod xc_private_method_lib; fn main() { let _ = xc_private_method_lib::Struct::static_meth_struct(); - //~^ ERROR: unresolved name + //~^ ERROR: method `static_meth_struct` is private let _ = xc_private_method_lib::Enum::static_meth_enum(); - //~^ ERROR: unresolved name + //~^ ERROR: method `static_meth_enum` is private } diff --git a/src/test/compile-fail/xcrate-private-by-default.rs b/src/test/compile-fail/xcrate-private-by-default.rs index 38649981f939c..ca1221e7432f5 100644 --- a/src/test/compile-fail/xcrate-private-by-default.rs +++ b/src/test/compile-fail/xcrate-private-by-default.rs @@ -10,15 +10,14 @@ // aux-build:static_priv_by_default.rs -#[allow(unused_imports)]; -#[no_std]; +#[no_std]; // helps if debugging resolve extern mod static_priv_by_default; fn foo() {} #[start] -fn main(_: int, _: **u8, _: *u8) -> int { +fn main(_: int, _: **u8) -> int { // Actual public items should be public static_priv_by_default::a; static_priv_by_default::b; @@ -33,25 +32,23 @@ fn main(_: int, _: **u8, _: *u8) -> int { // private items at the top should be inaccessible static_priv_by_default::i; - //~^ ERROR: unresolved name + //~^ ERROR: static `i` is private static_priv_by_default::j; - //~^ ERROR: unresolved name + //~^ ERROR: function `j` is private static_priv_by_default::k; - //~^ ERROR: unresolved name + //~^ ERROR: struct `k` is private foo::(); - //~^ ERROR: use of undeclared type name - //~^^ ERROR: use of undeclared type name + //~^ ERROR: type `l` is private // public items in a private mod should be inaccessible static_priv_by_default::foo::a; - //~^ ERROR: unresolved name + //~^ ERROR: static `a` is private static_priv_by_default::foo::b; - //~^ ERROR: unresolved name + //~^ ERROR: function `b` is private static_priv_by_default::foo::c; - //~^ ERROR: unresolved name + //~^ ERROR: struct `c` is private foo::(); - //~^ ERROR: use of undeclared type name - //~^^ ERROR: use of undeclared type name + //~^ ERROR: type `d` is private 3 } diff --git a/src/test/run-pass/export-non-interference2.rs b/src/test/run-pass/export-non-interference2.rs index 9147596b0db75..8fdc9c563156d 100644 --- a/src/test/run-pass/export-non-interference2.rs +++ b/src/test/run-pass/export-non-interference2.rs @@ -9,7 +9,7 @@ // except according to those terms. mod foo { - mod bar { + pub mod bar { pub fn y() { super::super::foo::x(); } } diff --git a/src/test/run-pass/foreign-dupe.rs b/src/test/run-pass/foreign-dupe.rs index 3ff1ebb57322c..e8a9d666dcc0c 100644 --- a/src/test/run-pass/foreign-dupe.rs +++ b/src/test/run-pass/foreign-dupe.rs @@ -17,7 +17,7 @@ mod rustrt1 { #[abi = "cdecl"] #[link_name = "rustrt"] extern { - fn rust_get_test_int() -> libc::intptr_t; + pub fn rust_get_test_int() -> libc::intptr_t; } } @@ -27,7 +27,7 @@ mod rustrt2 { #[abi = "cdecl"] #[link_name = "rustrt"] extern { - fn rust_get_test_int() -> libc::intptr_t; + pub fn rust_get_test_int() -> libc::intptr_t; } } diff --git a/src/test/run-pass/foreign-no-abi.rs b/src/test/run-pass/foreign-no-abi.rs index f9c2698eda499..6a7ee7101efcf 100644 --- a/src/test/run-pass/foreign-no-abi.rs +++ b/src/test/run-pass/foreign-no-abi.rs @@ -14,7 +14,7 @@ mod rustrt { use std::libc; extern { - fn rust_get_test_int() -> libc::intptr_t; + pub fn rust_get_test_int() -> libc::intptr_t; } } diff --git a/src/test/run-pass/intrinsic-uninit.rs b/src/test/run-pass/intrinsic-uninit.rs index 993e277719765..53c9ed631417c 100644 --- a/src/test/run-pass/intrinsic-uninit.rs +++ b/src/test/run-pass/intrinsic-uninit.rs @@ -11,7 +11,7 @@ mod rusti { #[abi = "rust-intrinsic"] extern "rust-intrinsic" { - fn uninit() -> T; + pub fn uninit() -> T; } } pub fn main() { diff --git a/src/test/run-pass/intrinsics-integer.rs b/src/test/run-pass/intrinsics-integer.rs index bbbc8bf4c1f79..2ca71866db8e0 100644 --- a/src/test/run-pass/intrinsics-integer.rs +++ b/src/test/run-pass/intrinsics-integer.rs @@ -17,24 +17,24 @@ extern mod extra; mod rusti { #[abi = "rust-intrinsic"] extern "rust-intrinsic" { - fn ctpop8(x: i8) -> i8; - fn ctpop16(x: i16) -> i16; - fn ctpop32(x: i32) -> i32; - fn ctpop64(x: i64) -> i64; - - fn ctlz8(x: i8) -> i8; - fn ctlz16(x: i16) -> i16; - fn ctlz32(x: i32) -> i32; - fn ctlz64(x: i64) -> i64; - - fn cttz8(x: i8) -> i8; - fn cttz16(x: i16) -> i16; - fn cttz32(x: i32) -> i32; - fn cttz64(x: i64) -> i64; - - fn bswap16(x: i16) -> i16; - fn bswap32(x: i32) -> i32; - fn bswap64(x: i64) -> i64; + pub fn ctpop8(x: i8) -> i8; + pub fn ctpop16(x: i16) -> i16; + pub fn ctpop32(x: i32) -> i32; + pub fn ctpop64(x: i64) -> i64; + + pub fn ctlz8(x: i8) -> i8; + pub fn ctlz16(x: i16) -> i16; + pub fn ctlz32(x: i32) -> i32; + pub fn ctlz64(x: i64) -> i64; + + pub fn cttz8(x: i8) -> i8; + pub fn cttz16(x: i16) -> i16; + pub fn cttz32(x: i32) -> i32; + pub fn cttz64(x: i64) -> i64; + + pub fn bswap16(x: i16) -> i16; + pub fn bswap32(x: i32) -> i32; + pub fn bswap64(x: i64) -> i64; } } diff --git a/src/test/run-pass/mod_dir_simple/load_another_mod.rs b/src/test/run-pass/mod_dir_simple/load_another_mod.rs index 335da61cd4ebb..c11b1e8c07469 100644 --- a/src/test/run-pass/mod_dir_simple/load_another_mod.rs +++ b/src/test/run-pass/mod_dir_simple/load_another_mod.rs @@ -8,4 +8,4 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -mod test; +pub mod test; diff --git a/src/test/run-pass/rec-align-u64.rs b/src/test/run-pass/rec-align-u64.rs index f3bfb998dbbc5..e833f67b51edc 100644 --- a/src/test/run-pass/rec-align-u64.rs +++ b/src/test/run-pass/rec-align-u64.rs @@ -46,7 +46,7 @@ mod m { } #[cfg(target_arch = "x86_64")] - mod m { + pub mod m { pub fn align() -> uint { 8u } pub fn size() -> uint { 16u } } From 2c76cdae3e091ee8fe662713e89a56ceffc6e19c Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 5 Oct 2013 17:07:57 -0700 Subject: [PATCH 3/4] Document visibility in the manual/tutorial This removes the warning "Note" about visibility not being fully defined, as it should now be considered fully defined with further bugs being considered just bugs in the implementation. --- doc/rust.md | 161 ++++++++++++++++++++++++++++++++++++++++++++++++ doc/tutorial.md | 25 ++++---- 2 files changed, 174 insertions(+), 12 deletions(-) diff --git a/doc/rust.md b/doc/rust.md index e998f97869f7a..98978e3e5a358 100644 --- a/doc/rust.md +++ b/doc/rust.md @@ -1501,6 +1501,167 @@ 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(); + } + } +} +~~~ + +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() {} +} +~~~ + +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} diff --git a/doc/tutorial.md b/doc/tutorial.md index 49ba38954b386..b2da355b12201 100644 --- a/doc/tutorial.md +++ b/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 From 7cd66924252a46c1b4524b9de4ad5e4cfc1c1faa Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 7 Oct 2013 13:01:47 -0700 Subject: [PATCH 4/4] Fix merge fallout of privacy changes --- doc/rust.md | 4 ++++ src/librustc/metadata/encoder.rs | 1 + src/librustc/middle/privacy.rs | 30 ++++++++++++++++++-------- src/librustc/middle/resolve.rs | 14 +++++++++--- src/librustc/middle/trans/context.rs | 1 - src/libstd/task/mod.rs | 2 +- src/test/compile-fail/glob-resolve1.rs | 2 ++ src/test/compile-fail/issue-4366-2.rs | 1 + src/test/compile-fail/privacy1.rs | 1 + src/test/compile-fail/privacy2.rs | 1 + src/test/compile-fail/privacy3.rs | 1 + src/test/compile-fail/privacy4.rs | 1 + src/test/run-pass/privacy1.rs | 29 +++++++++++++++++++++++++ 13 files changed, 74 insertions(+), 14 deletions(-) create mode 100644 src/test/run-pass/privacy1.rs diff --git a/doc/rust.md b/doc/rust.md index 98978e3e5a358..83833afb90e95 100644 --- a/doc/rust.md +++ b/doc/rust.md @@ -1624,6 +1624,8 @@ pub mod submodule { } } } + +# fn main() {} ~~~ For a rust program to pass the privacy checking pass, all paths must be valid @@ -1643,6 +1645,8 @@ pub use api = self::implementation; mod implementation { pub fn f() {} } + +# fn main() {} ~~~ This means that any external crate referencing `implementation::f` would receive diff --git a/src/librustc/metadata/encoder.rs b/src/librustc/metadata/encoder.rs index ad190dfcfb127..3f2c55a55b838 100644 --- a/src/librustc/metadata/encoder.rs +++ b/src/librustc/metadata/encoder.rs @@ -623,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 { diff --git a/src/librustc/middle/privacy.rs b/src/librustc/middle/privacy.rs index f395231124b64..fb4b76c7c916e 100644 --- a/src/librustc/middle/privacy.rs +++ b/src/librustc/middle/privacy.rs @@ -64,20 +64,27 @@ impl<'self> Visitor<()> for ParentVisitor<'self> { } } } + + // Trait methods are always considered "public", but if the trait is + // private then we need some private item in the chain from the + // method to the root. In this case, if the trait is private, then + // parent all the methods to the trait to indicate that they're + // private. + ast::item_trait(_, _, ref methods) if item.vis != ast::public => { + for m in methods.iter() { + match *m { + ast::provided(ref m) => self.parents.insert(m.id, item.id), + ast::required(ref m) => self.parents.insert(m.id, item.id), + }; + } + } + _ => {} } visit::walk_item(self, item, ()); self.curparent = prev; } - fn visit_trait_method(&mut self, m: &ast::trait_method, _: ()) { - match *m { - ast::provided(ref m) => self.parents.insert(m.id, self.curparent), - ast::required(ref m) => self.parents.insert(m.id, self.curparent), - }; - visit::walk_trait_method(self, m, ()); - } - fn visit_foreign_item(&mut self, a: @ast::foreign_item, _: ()) { self.parents.insert(a.id, self.curparent); visit::walk_foreign_item(self, a, ()); @@ -85,7 +92,12 @@ impl<'self> Visitor<()> for ParentVisitor<'self> { fn visit_fn(&mut self, a: &visit::fn_kind, b: &ast::fn_decl, c: &ast::Block, d: Span, id: ast::NodeId, _: ()) { - self.parents.insert(id, self.curparent); + // We already took care of some trait methods above, otherwise things + // like impl methods and pub trait methods are parented to the + // containing module, not the containing trait. + if !self.parents.contains_key(&id) { + self.parents.insert(id, self.curparent); + } visit::walk_fn(self, a, b, c, d, id, ()); } diff --git a/src/librustc/middle/resolve.rs b/src/librustc/middle/resolve.rs index 447f1075585c8..156cb741af3e0 100644 --- a/src/librustc/middle/resolve.rs +++ b/src/librustc/middle/resolve.rs @@ -1649,7 +1649,15 @@ impl Resolver { external crate) building external def, priv {:?}", vis); let is_public = vis == ast::public; - if is_public { + let is_exported = is_public && match new_parent { + ModuleReducedGraphParent(module) => { + match module.def_id { + None => true, + Some(did) => self.external_exports.contains(&did) + } + } + }; + if is_exported { self.external_exports.insert(def_id_of_def(def)); } match def { @@ -1725,7 +1733,7 @@ impl Resolver { if explicit_self != sty_static { interned_method_names.insert(method_name.name); } - if is_public { + if is_exported { self.external_exports.insert(method_def_id); } } @@ -1952,7 +1960,7 @@ impl Resolver { /// Builds the reduced graph rooted at the 'use' directive for an external /// crate. fn build_reduced_graph_for_external_crate(&mut self, - root: @mut Module) { + root: @mut Module) { do csearch::each_top_level_item_of_crate(self.session.cstore, root.def_id.unwrap().crate) |def_like, ident, visibility| { diff --git a/src/librustc/middle/trans/context.rs b/src/librustc/middle/trans/context.rs index 2b3874e0bf0d8..bb1e32bf34e3d 100644 --- a/src/librustc/middle/trans/context.rs +++ b/src/librustc/middle/trans/context.rs @@ -16,7 +16,6 @@ use lib::llvm::{llvm, TargetData, TypeNames}; use lib::llvm::mk_target_data; use metadata::common::LinkMeta; use middle::astencode; -use middle::privacy; use middle::resolve; use middle::trans::adt; use middle::trans::base; diff --git a/src/libstd/task/mod.rs b/src/libstd/task/mod.rs index cb45c6f78ebf5..a46e115a503ca 100644 --- a/src/libstd/task/mod.rs +++ b/src/libstd/task/mod.rs @@ -1069,7 +1069,7 @@ fn test_try_fail() { #[cfg(test)] fn get_sched_id() -> int { - do Local::borrow |sched: &mut ::rt::sched::Scheduler| { + do Local::borrow |sched: &mut ::rt::shouldnt_be_public::Scheduler| { sched.sched_id() as int } } diff --git a/src/test/compile-fail/glob-resolve1.rs b/src/test/compile-fail/glob-resolve1.rs index a0004f98ecf27..7363fb6d0b2df 100644 --- a/src/test/compile-fail/glob-resolve1.rs +++ b/src/test/compile-fail/glob-resolve1.rs @@ -10,6 +10,8 @@ // Make sure that globs only bring in public things. +#[feature(globs)]; + use bar::*; mod bar { diff --git a/src/test/compile-fail/issue-4366-2.rs b/src/test/compile-fail/issue-4366-2.rs index 4530267f35ff9..6764b489b6255 100644 --- a/src/test/compile-fail/issue-4366-2.rs +++ b/src/test/compile-fail/issue-4366-2.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#[feature(globs)]; // ensures that 'use foo:*' doesn't import non-public item diff --git a/src/test/compile-fail/privacy1.rs b/src/test/compile-fail/privacy1.rs index 2e2b53331cafa..0d4dbc86dce4d 100644 --- a/src/test/compile-fail/privacy1.rs +++ b/src/test/compile-fail/privacy1.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#[feature(globs)]; #[no_std]; // makes debugging this test *a lot* easier (during resolve) mod bar { diff --git a/src/test/compile-fail/privacy2.rs b/src/test/compile-fail/privacy2.rs index e8e21021cccd6..98772b0c67b82 100644 --- a/src/test/compile-fail/privacy2.rs +++ b/src/test/compile-fail/privacy2.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#[feature(globs)]; #[no_std]; // makes debugging this test *a lot* easier (during resolve) // Test to make sure that globs don't leak in regular `use` statements. diff --git a/src/test/compile-fail/privacy3.rs b/src/test/compile-fail/privacy3.rs index 523d4cd4b8ddd..3308be4a12e78 100644 --- a/src/test/compile-fail/privacy3.rs +++ b/src/test/compile-fail/privacy3.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#[feature(globs)]; #[no_std]; // makes debugging this test *a lot* easier (during resolve) // Test to make sure that private items imported through globs remain private diff --git a/src/test/compile-fail/privacy4.rs b/src/test/compile-fail/privacy4.rs index 88a9b2f30580b..4e33536b2b05f 100644 --- a/src/test/compile-fail/privacy4.rs +++ b/src/test/compile-fail/privacy4.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +#[feature(globs)]; #[no_std]; // makes debugging this test *a lot* easier (during resolve) // Test to make sure that private items imported through globs remain private diff --git a/src/test/run-pass/privacy1.rs b/src/test/run-pass/privacy1.rs new file mode 100644 index 0000000000000..7a07c97090223 --- /dev/null +++ b/src/test/run-pass/privacy1.rs @@ -0,0 +1,29 @@ +// Copyright 2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +pub mod test2 { + // This used to generate an ICE (make sure that default functions are + // parented to their trait to find the first private thing as the trait). + + struct B; + trait A { fn foo(&self) {} } + impl A for B {} + + mod tests { + use super::A; + fn foo() { + let a = super::B; + a.foo(); + } + } +} + + +pub fn main() {}