diff options
40 files changed, 307 insertions, 506 deletions
diff --git a/src/librustc/front/map/mod.rs b/src/librustc/front/map/mod.rs index adf14d0d89c..44f588c2e9c 100644 --- a/src/librustc/front/map/mod.rs +++ b/src/librustc/front/map/mod.rs @@ -495,6 +495,30 @@ impl<'ast> Map<'ast> { } } + /// Returns the NodeId of `id`'s nearest module parent, or `id` itself if no + /// module parent is in this map. + fn get_module_parent(&self, id: NodeId) -> NodeId { + match self.walk_parent_nodes(id, |node| match *node { + NodeItem(&Item { node: Item_::ItemMod(_), .. }) => true, + _ => false, + }) { + Ok(id) => id, + Err(id) => id, + } + } + + pub fn private_item_is_visible_from(&self, item: NodeId, block: NodeId) -> bool { + // A private item is visible from everything in its nearest module parent. + let visibility = self.get_module_parent(item); + let mut block_ancestor = self.get_module_parent(block); + loop { + if block_ancestor == visibility { return true } + let block_ancestor_parent = self.get_module_parent(block_ancestor); + if block_ancestor_parent == block_ancestor { return false } + block_ancestor = block_ancestor_parent; + } + } + /// Returns the nearest enclosing scope. A scope is an item or block. /// FIXME it is not clear to me that all items qualify as scopes - statics /// and associated types probably shouldn't, for example. Behaviour in this diff --git a/src/librustc/middle/def.rs b/src/librustc/middle/def.rs index aee8fb10c2a..6d4799749b9 100644 --- a/src/librustc/middle/def.rs +++ b/src/librustc/middle/def.rs @@ -9,7 +9,6 @@ // except according to those terms. use middle::def_id::DefId; -use middle::privacy::LastPrivate; use middle::subst::ParamSpace; use util::nodemap::NodeMap; use syntax::ast; @@ -65,7 +64,6 @@ pub enum Def { #[derive(Copy, Clone, Debug)] pub struct PathResolution { pub base_def: Def, - pub last_private: LastPrivate, pub depth: usize } @@ -84,12 +82,10 @@ impl PathResolution { } pub fn new(base_def: Def, - last_private: LastPrivate, depth: usize) -> PathResolution { PathResolution { base_def: base_def, - last_private: last_private, depth: depth, } } @@ -152,4 +148,29 @@ impl Def { _ => None } } + + pub fn kind_name(&self) -> &'static str { + match *self { + Def::Fn(..) => "function", + Def::Mod(..) => "module", + Def::ForeignMod(..) => "foreign module", + Def::Static(..) => "static", + Def::Variant(..) => "variant", + Def::Enum(..) => "enum", + Def::TyAlias(..) => "type", + Def::AssociatedTy(..) => "associated type", + Def::Struct(..) => "struct", + Def::Trait(..) => "trait", + Def::Method(..) => "method", + Def::Const(..) => "const", + Def::AssociatedConst(..) => "associated const", + Def::TyParam(..) => "type parameter", + Def::PrimTy(..) => "builtin type", + Def::Local(..) => "local variable", + Def::Upvar(..) => "closure capture", + Def::Label(..) => "label", + Def::SelfTy(..) => "self type", + Def::Err => "unresolved item", + } + } } diff --git a/src/librustc/middle/privacy.rs b/src/librustc/middle/privacy.rs index f464ea58c2d..c1dc727449a 100644 --- a/src/librustc/middle/privacy.rs +++ b/src/librustc/middle/privacy.rs @@ -12,11 +12,6 @@ //! outside their scopes. This pass will also generate a set of exported items //! which are available for use externally when compiled as a library. -pub use self::PrivateDep::*; -pub use self::ImportUse::*; -pub use self::LastPrivate::*; - -use middle::def_id::DefId; use util::nodemap::{DefIdSet, FnvHashMap}; use std::hash::Hash; @@ -64,39 +59,3 @@ impl<Id: Hash + Eq> Default for AccessLevels<Id> { /// A set containing all exported definitions from external crates. /// The set does not contain any entries from local crates. pub type ExternalExports = DefIdSet; - -#[derive(Copy, Clone, Debug)] -pub enum LastPrivate { - LastMod(PrivateDep), - // `use` directives (imports) can refer to two separate definitions in the - // type and value namespaces. We record here the last private node for each - // and whether the import is in fact used for each. - // If the Option<PrivateDep> fields are None, it means there is no definition - // in that namespace. - LastImport{value_priv: Option<PrivateDep>, - value_used: ImportUse, - type_priv: Option<PrivateDep>, - type_used: ImportUse}, -} - -#[derive(Copy, Clone, Debug)] -pub enum PrivateDep { - AllPublic, - DependsOn(DefId), -} - -// How an import is used. -#[derive(Copy, Clone, PartialEq, Debug)] -pub enum ImportUse { - Unused, // The import is not used. - Used, // The import is used. -} - -impl LastPrivate { - pub fn or(self, other: LastPrivate) -> LastPrivate { - match (self, other) { - (me, LastMod(AllPublic)) => me, - (_, other) => other, - } - } -} diff --git a/src/librustc/middle/ty/mod.rs b/src/librustc/middle/ty/mod.rs index 5d61892fbe2..105e0bb7b15 100644 --- a/src/librustc/middle/ty/mod.rs +++ b/src/librustc/middle/ty/mod.rs @@ -173,6 +173,14 @@ impl<'tcx> ImplOrTraitItem<'tcx> { } } + pub fn def(&self) -> Def { + match *self { + ConstTraitItem(ref associated_const) => Def::AssociatedConst(associated_const.def_id), + MethodTraitItem(ref method) => Def::Method(method.def_id), + TypeTraitItem(ref ty) => Def::AssociatedTy(ty.container.id(), ty.def_id), + } + } + pub fn def_id(&self) -> DefId { match *self { ConstTraitItem(ref associated_const) => associated_const.def_id, diff --git a/src/librustc_metadata/astencode.rs b/src/librustc_metadata/astencode.rs index fe4df865a0e..ff14652477e 100644 --- a/src/librustc_metadata/astencode.rs +++ b/src/librustc_metadata/astencode.rs @@ -32,7 +32,6 @@ use middle::ty::cast; use middle::const_qualif::ConstQualif; use middle::def::{self, Def}; use middle::def_id::DefId; -use middle::privacy::{AllPublic, LastMod}; use middle::region; use middle::subst; use middle::ty::{self, Ty}; @@ -254,7 +253,7 @@ trait def_id_encoder_helpers { } impl<S:serialize::Encoder> def_id_encoder_helpers for S - where <S as serialize::serialize::Encoder>::Error: Debug + where <S as serialize::Encoder>::Error: Debug { fn emit_def_id(&mut self, did: DefId) { did.encode(self).unwrap() @@ -268,7 +267,7 @@ trait def_id_decoder_helpers { } impl<D:serialize::Decoder> def_id_decoder_helpers for D - where <D as serialize::serialize::Decoder>::Error: Debug + where <D as serialize::Decoder>::Error: Debug { fn read_def_id(&mut self, dcx: &DecodeContext) -> DefId { let did: DefId = Decodable::decode(self).unwrap(); @@ -1161,8 +1160,6 @@ fn decode_side_tables(dcx: &DecodeContext, let def = decode_def(dcx, val_dsr); dcx.tcx.def_map.borrow_mut().insert(id, def::PathResolution { base_def: def, - // This doesn't matter cross-crate. - last_private: LastMod(AllPublic), depth: 0 }); } diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 2e7ace1b6a7..098c77a1bde 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -41,9 +41,6 @@ use rustc::lint; use rustc::middle::def::{self, Def}; use rustc::middle::def_id::DefId; use rustc::middle::privacy::{AccessLevel, AccessLevels}; -use rustc::middle::privacy::ImportUse::*; -use rustc::middle::privacy::LastPrivate::*; -use rustc::middle::privacy::PrivateDep::*; use rustc::middle::privacy::ExternalExports; use rustc::middle::ty; use rustc::util::nodemap::{NodeMap, NodeSet}; @@ -692,32 +689,7 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> { /// 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).unwrap(); - debug!("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 { - debug!("privacy - questioning {}, {}", self.nodestr(cur), 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 - _ => {} - } - - cur = *self.parents.get(&cur).unwrap(); - } + self.tcx.map.private_item_is_visible_from(id, self.curitem) } fn report_error(&self, result: CheckResult) -> bool { @@ -743,7 +715,6 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> { source_did: Option<DefId>, msg: &str) -> CheckResult { - use rustc_front::hir::Item_::ItemExternCrate; debug!("ensure_public(span={:?}, to_check={:?}, source_did={:?}, msg={:?})", span, to_check, source_did, msg); let def_privacy = self.def_privacy(to_check); @@ -765,20 +736,6 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> { let def_id = source_did.unwrap_or(to_check); let node_id = self.tcx.map.as_local_node_id(def_id); - // Warn when using a inaccessible extern crate. - if let Some(node_id) = self.tcx.map.as_local_node_id(to_check) { - match self.tcx.map.get(node_id) { - ast_map::Node::NodeItem(&hir::Item { node: ItemExternCrate(_), name, .. }) => { - self.tcx.sess.add_lint(lint::builtin::INACCESSIBLE_EXTERN_CRATE, - node_id, - span, - format!("extern crate `{}` is private", name)); - return None; - } - _ => {} - } - } - let (err_span, err_msg) = if Some(id) == node_id { return Some((span, format!("{} is private", msg), None)); } else { @@ -835,7 +792,7 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> { } UnnamedField(idx) => &v.fields[idx] }; - if field.vis == hir::Public || self.local_private_accessible(field.did) { + if field.vis == hir::Public || self.local_private_accessible(def.did) { return; } @@ -867,100 +824,6 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> { name))); } - // Checks that a path is in scope. - fn check_path(&mut self, span: Span, path_id: ast::NodeId, last: ast::Name) { - debug!("privacy - path {}", self.nodestr(path_id)); - let path_res = *self.tcx.def_map.borrow().get(&path_id).unwrap(); - let ck = |tyname: &str| { - let ck_public = |def: DefId| { - debug!("privacy - ck_public {:?}", def); - let origdid = path_res.def_id(); - self.ensure_public(span, - def, - Some(origdid), - &format!("{} `{}`", tyname, last)) - }; - - match path_res.last_private { - LastMod(AllPublic) => {}, - LastMod(DependsOn(def)) => { - self.report_error(ck_public(def)); - }, - LastImport { value_priv, - value_used: check_value, - type_priv, - type_used: check_type } => { - // This dance with found_error is because we don't want to - // report a privacy error twice for the same directive. - let found_error = match (type_priv, check_type) { - (Some(DependsOn(def)), Used) => { - !self.report_error(ck_public(def)) - }, - _ => false, - }; - if !found_error { - match (value_priv, check_value) { - (Some(DependsOn(def)), Used) => { - self.report_error(ck_public(def)); - }, - _ => {}, - } - } - // If an import is not used in either namespace, we still - // want to check that it could be legal. Therefore we check - // in both namespaces and only report an error if both would - // be illegal. We only report one error, even if it is - // illegal to import from both namespaces. - match (value_priv, check_value, type_priv, check_type) { - (Some(p), Unused, None, _) | - (None, _, Some(p), Unused) => { - let p = match p { - AllPublic => None, - DependsOn(def) => ck_public(def), - }; - if p.is_some() { - self.report_error(p); - } - }, - (Some(v), Unused, Some(t), Unused) => { - let v = match v { - AllPublic => None, - DependsOn(def) => ck_public(def), - }; - let t = match t { - AllPublic => None, - DependsOn(def) => ck_public(def), - }; - if let (Some(_), Some(t)) = (v, t) { - self.report_error(Some(t)); - } - }, - _ => {}, - } - }, - } - }; - // FIXME(#12334) Imports can refer to definitions in both the type and - // value namespaces. The privacy information is aware of this, but the - // def map is not. Therefore the names we work out below will not always - // be accurate and we can get slightly wonky error messages (but type - // checking is always correct). - match path_res.full_def() { - Def::Fn(..) => ck("function"), - Def::Static(..) => ck("static"), - Def::Const(..) => ck("const"), - Def::AssociatedConst(..) => ck("associated const"), - Def::Variant(..) => ck("variant"), - Def::TyAlias(..) => ck("type"), - Def::Enum(..) => ck("enum"), - Def::Trait(..) => ck("trait"), - Def::Struct(..) => ck("struct"), - Def::Method(..) => ck("method"), - Def::Mod(..) => ck("module"), - _ => {} - } - } - // Checks that a method is in scope. fn check_method(&mut self, span: Span, method_def_id: DefId, name: ast::Name) { @@ -1036,7 +899,7 @@ impl<'a, 'tcx, 'v> Visitor<'v> for PrivacyVisitor<'a, 'tcx> { _ => expr_ty }.ty_adt_def().unwrap(); let any_priv = def.struct_variant().fields.iter().any(|f| { - f.vis != hir::Public && !self.local_private_accessible(f.did) + f.vis != hir::Public && !self.local_private_accessible(def.did) }); if any_priv { span_err!(self.tcx.sess, expr.span, E0450, @@ -1102,25 +965,6 @@ impl<'a, 'tcx, 'v> Visitor<'v> for PrivacyVisitor<'a, 'tcx> { intravisit::walk_foreign_item(self, fi); self.in_foreign = false; } - - fn visit_path(&mut self, path: &hir::Path, id: ast::NodeId) { - if !path.segments.is_empty() { - self.check_path(path.span, id, path.segments.last().unwrap().identifier.name); - intravisit::walk_path(self, path); - } - } - - fn visit_path_list_item(&mut self, prefix: &hir::Path, item: &hir::PathListItem) { - let name = if let hir::PathListIdent { name, .. } = item.node { - name - } else if !prefix.segments.is_empty() { - prefix.segments.last().unwrap().identifier.name - } else { - self.tcx.sess.bug("`self` import in an import list with empty prefix"); - }; - self.check_path(item.span, item.node.id(), name); - intravisit::walk_path_list_item(self, prefix, item); - } } //////////////////////////////////////////////////////////////////////////////// diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 758611a1ec2..dbbaf0e23c7 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -291,10 +291,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { self.external_exports.insert(def_id); let parent_link = ModuleParentLink(parent, name); let def = Def::Mod(def_id); - let local_def_id = self.ast_map.local_def_id(item.id); - let external_module = - self.new_extern_crate_module(parent_link, def, is_public, local_def_id); - self.define(parent, name, TypeNS, (external_module, sp)); + let module = self.new_extern_crate_module(parent_link, def, is_public, item.id); + self.define(parent, name, TypeNS, (module, sp)); if is_public { let export = Export { name: name, def_id: def_id }; @@ -304,7 +302,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { } } - self.build_reduced_graph_for_external_crate(external_module); + self.build_reduced_graph_for_external_crate(module); } parent } @@ -494,7 +492,7 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> { debug!("(building reduced graph for external crate) building external def {}, priv {:?}", final_ident, vis); - let is_public = vis == hir::Public; + let is_public = vis == hir::Public || new_parent.is_trait(); let mut modifiers = DefModifiers::empty(); if is_public { diff --git a/src/librustc_resolve/check_unused.rs b/src/librustc_resolve/check_unused.rs index 178e2a4d1bc..ea197109cab 100644 --- a/src/librustc_resolve/check_unused.rs +++ b/src/librustc_resolve/check_unused.rs @@ -23,7 +23,6 @@ use Resolver; use Namespace::{TypeNS, ValueNS}; use rustc::lint; -use rustc::middle::privacy::{DependsOn, LastImport, Used, Unused}; use syntax::ast; use syntax::codemap::{Span, DUMMY_SP}; @@ -69,45 +68,6 @@ impl<'a, 'b, 'tcx> UnusedImportCheckVisitor<'a, 'b, 'tcx> { span, "unused import".to_string()); } - - let mut def_map = self.def_map.borrow_mut(); - let path_res = if let Some(r) = def_map.get_mut(&id) { - r - } else { - return; - }; - let (v_priv, t_priv) = match path_res.last_private { - LastImport { value_priv, type_priv, .. } => (value_priv, type_priv), - _ => { - panic!("we should only have LastImport for `use` directives") - } - }; - - let mut v_used = if self.used_imports.contains(&(id, ValueNS)) { - Used - } else { - Unused - }; - let t_used = if self.used_imports.contains(&(id, TypeNS)) { - Used - } else { - Unused - }; - - match (v_priv, t_priv) { - // Since some items may be both in the value _and_ type namespaces (e.g., structs) - // we might have two LastPrivates pointing at the same thing. There is no point - // checking both, so lets not check the value one. - (Some(DependsOn(def_v)), Some(DependsOn(def_t))) if def_v == def_t => v_used = Unused, - _ => {} - } - - path_res.last_private = LastImport { - value_priv: v_priv, - value_used: v_used, - type_priv: t_priv, - type_used: t_used, - }; } } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index e139ac8b2a0..a160e1384d1 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -54,7 +54,7 @@ use rustc::middle::cstore::{CrateStore, DefLike, DlDef}; use rustc::middle::def::*; use rustc::middle::def_id::DefId; use rustc::middle::pat_util::pat_bindings; -use rustc::middle::privacy::*; +use rustc::middle::privacy::ExternalExports; use rustc::middle::subst::{ParamSpace, FnSpace, TypeSpace}; use rustc::middle::ty::{Freevar, FreevarMap, TraitMap, GlobMap}; use rustc::util::nodemap::{NodeMap, DefIdSet, FnvHashMap}; @@ -757,8 +757,8 @@ enum AssocItemResolveResult { #[derive(Copy, Clone)] enum BareIdentifierPatternResolution { - FoundStructOrEnumVariant(Def, LastPrivate), - FoundConst(Def, LastPrivate, Name), + FoundStructOrEnumVariant(Def), + FoundConst(Def, Name), BareIdentifierPatternUnresolved, } @@ -807,9 +807,9 @@ pub struct ModuleS<'a> { def: Option<Def>, is_public: bool, - // If the module is an extern crate, `def` is root of the external crate and `extern_crate_did` - // is the DefId of the local `extern crate` item (otherwise, `extern_crate_did` is None). - extern_crate_did: Option<DefId>, + // If the module is an extern crate, `def` is root of the external crate and `extern_crate_id` + // is the NodeId of the local `extern crate` item (otherwise, `extern_crate_id` is None). + extern_crate_id: Option<NodeId>, resolutions: RefCell<HashMap<(Name, Namespace), NameResolution<'a>>>, unresolved_imports: RefCell<Vec<ImportDirective>>, @@ -856,7 +856,7 @@ impl<'a> ModuleS<'a> { parent_link: parent_link, def: def, is_public: is_public, - extern_crate_did: None, + extern_crate_id: None, resolutions: RefCell::new(HashMap::new()), unresolved_imports: RefCell::new(Vec::new()), module_children: RefCell::new(NodeMap()), @@ -920,16 +920,6 @@ impl<'a> ModuleS<'a> { self.def.as_ref().map(Def::def_id) } - // This returns the DefId of the crate local item that controls this module's visibility. - // It is only used to compute `LastPrivate` data, and it differs from `def_id` only for extern - // crates, whose `def_id` is the external crate's root, not the local `extern crate` item. - fn local_def_id(&self) -> Option<DefId> { - match self.extern_crate_did { - Some(def_id) => Some(def_id), - None => self.def_id(), - } - } - fn is_normal(&self) -> bool { match self.def { Some(Def::Mod(_)) | Some(Def::ForeignMod(_)) => true, @@ -944,6 +934,15 @@ impl<'a> ModuleS<'a> { } } + fn is_ancestor_of(&self, module: Module<'a>) -> bool { + if self.def_id() == module.def_id() { return true } + match module.parent_link { + ParentLink::BlockParentLink(parent, _) | + ParentLink::ModuleParentLink(parent, _) => self.is_ancestor_of(parent), + _ => false, + } + } + pub fn inc_glob_count(&self) { self.glob_count.set(self.glob_count.get() + 1); } @@ -1010,9 +1009,14 @@ enum NameBindingKind<'a> { Import { binding: &'a NameBinding<'a>, id: NodeId, + // Some(error) if using this imported name causes the import to be a privacy error + privacy_error: Option<Box<PrivacyError<'a>>>, }, } +#[derive(Clone, Debug)] +struct PrivacyError<'a>(Span, Name, &'a NameBinding<'a>); + impl<'a> NameBinding<'a> { fn create_from_module(module: Module<'a>, span: Option<Span>) -> Self { let modifiers = if module.is_public { @@ -1040,14 +1044,6 @@ impl<'a> NameBinding<'a> { } } - fn local_def_id(&self) -> Option<DefId> { - match self.kind { - NameBindingKind::Def(def) => Some(def.def_id()), - NameBindingKind::Module(ref module) => module.local_def_id(), - NameBindingKind::Import { binding, .. } => binding.local_def_id(), - } - } - fn defined_with(&self, modifiers: DefModifiers) -> bool { self.modifiers.contains(modifiers) } @@ -1056,15 +1052,8 @@ impl<'a> NameBinding<'a> { self.defined_with(DefModifiers::PUBLIC) } - fn def_and_lp(&self) -> (Def, LastPrivate) { - let def = self.def().unwrap(); - if let Def::Err = def { return (def, LastMod(AllPublic)) } - let lp = if self.is_public() { AllPublic } else { DependsOn(self.local_def_id().unwrap()) }; - (def, LastMod(lp)) - } - fn is_extern_crate(&self) -> bool { - self.module().and_then(|module| module.extern_crate_did).is_some() + self.module().and_then(|module| module.extern_crate_id).is_some() } fn is_import(&self) -> bool { @@ -1170,6 +1159,7 @@ pub struct Resolver<'a, 'tcx: 'a> { // The intention is that the callback modifies this flag. // Once set, the resolver falls out of the walk, preserving the ribs. resolved: bool, + privacy_errors: Vec<PrivacyError<'a>>, arenas: &'a ResolverArenas<'a>, } @@ -1234,6 +1224,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { callback: None, resolved: false, + privacy_errors: Vec::new(), arenas: arenas, } @@ -1262,10 +1253,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { parent_link: ParentLink<'a>, def: Def, is_public: bool, - local_def: DefId) + local_node_id: NodeId) -> Module<'a> { let mut module = ModuleS::new(parent_link, Some(def), false, is_public); - module.extern_crate_did = Some(local_def); + module.extern_crate_id = Some(local_node_id); self.arenas.modules.alloc(module) } @@ -1280,12 +1271,15 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { self.used_crates.insert(krate); } - let import_id = match binding.kind { - NameBindingKind::Import { id, .. } => id, + let (import_id, privacy_error) = match binding.kind { + NameBindingKind::Import { id, ref privacy_error, .. } => (id, privacy_error), _ => return, }; self.used_imports.insert((import_id, ns)); + if let Some(error) = privacy_error.as_ref() { + self.privacy_errors.push((**error).clone()); + } if !self.make_glob_map { return; @@ -1313,9 +1307,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { module_: Module<'a>, module_path: &[Name], index: usize, - span: Span, - lp: LastPrivate) - -> ResolveResult<(Module<'a>, LastPrivate)> { + span: Span) + -> ResolveResult<Module<'a>> { fn search_parent_externals(needle: Name, module: Module) -> Option<Module> { match module.resolve_name(needle, TypeNS, false) { Success(binding) if binding.is_extern_crate() => Some(module), @@ -1331,7 +1324,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { 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 @@ -1379,15 +1371,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { // Check to see whether there are type bindings, and, if // so, whether there is a module within. if let Some(module_def) = binding.module() { + self.check_privacy(search_module, name, binding, span); search_module = module_def; - - // Keep track of the closest private module used - // when resolving this import chain. - if !binding.is_public() { - if let Some(did) = search_module.local_def_id() { - closest_private = LastMod(DependsOn(did)); - } - } } else { let msg = format!("Not a module `{}`", name); return Failed(Some((span, msg))); @@ -1398,7 +1383,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { index += 1; } - return Success((search_module, closest_private)); + return Success(search_module); } /// Attempts to resolve the module part of an import directive or path @@ -1411,9 +1396,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { module_path: &[Name], use_lexical_scope: UseLexicalScopeFlag, span: Span) - -> ResolveResult<(Module<'a>, LastPrivate)> { + -> ResolveResult<Module<'a>> { if module_path.len() == 0 { - return Success((self.graph_root, LastMod(AllPublic))) // Use the crate root + return Success(self.graph_root) // Use the crate root } debug!("(resolving module path for import) processing `{}` rooted at `{}`", @@ -1425,7 +1410,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { let search_module; let start_index; - let last_private; match module_prefix_result { Failed(None) => { let mpath = names_to_string(module_path); @@ -1459,7 +1443,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { // resolution process at index zero. search_module = self.graph_root; start_index = 0; - last_private = LastMod(AllPublic); } UseLexicalScope => { // This is not a crate-relative path. We resolve the @@ -1478,7 +1461,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { Some(containing_module) => { search_module = containing_module; start_index = 1; - last_private = LastMod(AllPublic); } None => return Failed(None), } @@ -1489,16 +1471,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { Success(PrefixFound(ref containing_module, index)) => { search_module = containing_module; start_index = index; - last_private = LastMod(DependsOn(containing_module.local_def_id() - .unwrap())); } } self.resolve_module_path_from_root(search_module, module_path, start_index, - span, - last_private) + span) } /// Invariant: This must only be called during main resolution, not during @@ -1851,8 +1830,8 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { match self.resolve_crate_relative_path(prefix.span, &prefix.segments, TypeNS) { - Some((def, lp)) => - self.record_def(item.id, PathResolution::new(def, lp, 0)), + Some(def) => + self.record_def(item.id, PathResolution::new(def, 0)), None => { resolve_error(self, prefix.span, @@ -2406,7 +2385,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { match self.resolve_bare_identifier_pattern(ident.unhygienic_name, pattern.span) { - FoundStructOrEnumVariant(def, lp) if const_ok => { + FoundStructOrEnumVariant(def) if const_ok => { debug!("(resolving pattern) resolving `{}` to struct or enum variant", renamed); @@ -2416,7 +2395,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { self.record_def(pattern.id, PathResolution { base_def: def, - last_private: lp, depth: 0, }); } @@ -2429,18 +2407,17 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { ); self.record_def(pattern.id, err_path_resolution()); } - FoundConst(def, lp, _) if const_ok => { + FoundConst(def, _) if const_ok => { debug!("(resolving pattern) resolving `{}` to constant", renamed); self.enforce_default_binding_mode(pattern, binding_mode, "a constant"); self.record_def(pattern.id, PathResolution { base_def: def, - last_private: lp, depth: 0, }); } - FoundConst(def, _, name) => { + FoundConst(def, name) => { resolve_error( self, pattern.span, @@ -2462,7 +2439,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { self.record_def(pattern.id, PathResolution { base_def: def, - last_private: LastMod(AllPublic), depth: 0, }); @@ -2680,10 +2656,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { // considered as not having a private component because // the lookup happened only within the current module. Some(def @ Def::Variant(..)) | Some(def @ Def::Struct(..)) => { - return FoundStructOrEnumVariant(def, LastMod(AllPublic)); + return FoundStructOrEnumVariant(def); } Some(def @ Def::Const(..)) | Some(def @ Def::AssociatedConst(..)) => { - return FoundConst(def, LastMod(AllPublic), name); + return FoundConst(def, name); } Some(Def::Static(..)) => { resolve_error(self, span, ResolutionError::StaticVariableReference); @@ -2764,7 +2740,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { let span = path.span; let segments = &path.segments[..path.segments.len() - path_depth]; - let mk_res = |(def, lp)| PathResolution::new(def, lp, path_depth); + let mk_res = |def| PathResolution::new(def, path_depth); if path.global { let def = self.resolve_crate_relative_path(span, segments, namespace); @@ -2777,14 +2753,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { let unqualified_def = self.resolve_identifier(last_ident, namespace, check_ribs, true); return unqualified_def.and_then(|def| self.adjust_local_def(def, span)) .map(|def| { - PathResolution::new(def, LastMod(AllPublic), path_depth) + PathResolution::new(def, path_depth) }); } let unqualified_def = self.resolve_identifier(last_ident, namespace, check_ribs, false); let def = self.resolve_module_relative_path(span, segments, namespace); match (def, unqualified_def) { - (Some((ref d, _)), Some(ref ud)) if *d == ud.def => { + (Some(d), Some(ref ud)) if d == ud.def => { self.session .add_lint(lint::builtin::UNUSED_QUALIFICATIONS, id, @@ -2931,7 +2907,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { span: Span, segments: &[hir::PathSegment], namespace: Namespace) - -> Option<(Def, LastPrivate)> { + -> Option<Def> { let module_path = segments.split_last() .unwrap() .1 @@ -2940,7 +2916,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { .collect::<Vec<_>>(); let containing_module; - let last_private; let current_module = self.current_module; match self.resolve_module_path(current_module, &module_path, UseLexicalScope, span) { Failed(err) => { @@ -2957,22 +2932,17 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { return None; } Indeterminate => return None, - Success((resulting_module, resulting_last_private)) => { + Success(resulting_module) => { containing_module = resulting_module; - last_private = resulting_last_private; } } let name = segments.last().unwrap().identifier.name; let result = self.resolve_name_in_module(containing_module, name, namespace, false, true); - let def = match result { - Success(binding) => { - let (def, lp) = binding.def_and_lp(); - (def, last_private.or(lp)) - } - _ => return None, - }; - return Some(def); + result.success().map(|binding| { + self.check_privacy(containing_module, name, binding, span); + binding.def().unwrap() + }) } /// Invariant: This must be called only during main resolution, not during @@ -2981,7 +2951,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { span: Span, segments: &[hir::PathSegment], namespace: Namespace) - -> Option<(Def, LastPrivate)> { + -> Option<Def> { let module_path = segments.split_last() .unwrap() .1 @@ -2992,12 +2962,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { let root_module = self.graph_root; let containing_module; - let last_private; match self.resolve_module_path_from_root(root_module, &module_path, 0, - span, - LastMod(AllPublic)) { + span) { Failed(err) => { let (span, msg) = match err { Some((span, msg)) => (span, msg), @@ -3014,20 +2982,17 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { Indeterminate => return None, - Success((resulting_module, resulting_last_private)) => { + Success(resulting_module) => { containing_module = resulting_module; - last_private = resulting_last_private; } } let name = segments.last().unwrap().identifier.name; - match self.resolve_name_in_module(containing_module, name, namespace, false, true) { - Success(binding) => { - let (def, lp) = binding.def_and_lp(); - Some((def, last_private.or(lp))) - } - _ => None, - } + let result = self.resolve_name_in_module(containing_module, name, namespace, false, true); + result.success().map(|binding| { + self.check_privacy(containing_module, name, binding, span); + binding.def().unwrap() + }) } fn resolve_identifier_in_local_ribs(&mut self, @@ -3116,10 +3081,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { .and_then(NameBinding::module) } } else { - match this.resolve_module_path(root, &name_path, UseLexicalScope, span) { - Success((module, _)) => Some(module), - _ => None, - } + this.resolve_module_path(root, &name_path, UseLexicalScope, span).success() } } @@ -3431,7 +3393,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { self.record_def(expr.id, PathResolution { base_def: def, - last_private: LastMod(AllPublic), depth: 0, }) } @@ -3624,12 +3585,6 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { fn record_def(&mut self, node_id: NodeId, resolution: PathResolution) { debug!("(recording def) recording {:?} for {}", resolution, node_id); - assert!(match resolution.last_private { - LastImport{..} => false, - _ => true, - }, - "Import should only be used for `use` directives"); - if let Some(prev_res) = self.def_map.borrow_mut().insert(node_id, resolution) { let span = self.ast_map.opt_span(node_id).unwrap_or(codemap::DUMMY_SP); self.session.span_bug(span, @@ -3652,6 +3607,37 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { } } } + + fn is_visible(&self, binding: &'a NameBinding<'a>, parent: Module<'a>) -> bool { + binding.is_public() || parent.is_ancestor_of(self.current_module) + } + + fn check_privacy(&mut self, + module: Module<'a>, + name: Name, + binding: &'a NameBinding<'a>, + span: Span) { + if !self.is_visible(binding, module) { + self.privacy_errors.push(PrivacyError(span, name, binding)); + } + } + + fn report_privacy_errors(&self) { + if self.privacy_errors.len() == 0 { return } + let mut reported_spans = HashSet::new(); + for &PrivacyError(span, name, binding) in &self.privacy_errors { + if !reported_spans.insert(span) { continue } + if binding.is_extern_crate() { + // Warn when using an inaccessible extern crate. + let node_id = binding.module().unwrap().extern_crate_id.unwrap(); + let msg = format!("extern crate `{}` is private", name); + self.session.add_lint(lint::builtin::INACCESSIBLE_EXTERN_CRATE, node_id, span, msg); + } else { + let def = binding.def().unwrap(); + self.session.span_err(span, &format!("{} `{}` is private", def.kind_name(), name)); + } + } + } } @@ -3767,7 +3753,6 @@ fn module_to_string(module: Module) -> String { fn err_path_resolution() -> PathResolution { PathResolution { base_def: Def::Err, - last_private: LastMod(AllPublic), depth: 0, } } @@ -3809,6 +3794,7 @@ pub fn resolve_crate<'a, 'tcx>(session: &'a Session, resolver.resolve_crate(krate); check_unused::check_crate(&mut resolver, krate); + resolver.report_privacy_errors(); CrateMap { def_map: resolver.def_map, diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index c068ff258b0..f6d23c8caa2 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -13,7 +13,7 @@ use self::ImportDirectiveSubclass::*; use DefModifiers; use Module; use Namespace::{self, TypeNS, ValueNS}; -use {NameBinding, NameBindingKind}; +use {NameBinding, NameBindingKind, PrivacyError}; use ResolveResult; use ResolveResult::*; use Resolver; @@ -25,7 +25,6 @@ use build_reduced_graph; use rustc::lint; use rustc::middle::def::*; -use rustc::middle::privacy::*; use syntax::ast::{NodeId, Name}; use syntax::attr::AttrMetaMethods; @@ -79,7 +78,9 @@ impl ImportDirective { // Given the binding to which this directive resolves in a particular namespace, // this returns the binding for the name this directive defines in that namespace. - fn import<'a>(&self, binding: &'a NameBinding<'a>) -> NameBinding<'a> { + fn import<'a>(&self, + binding: &'a NameBinding<'a>, + privacy_error: Option<Box<PrivacyError<'a>>>) -> NameBinding<'a> { let mut modifiers = match self.is_public { true => DefModifiers::PUBLIC | DefModifiers::IMPORTABLE, false => DefModifiers::empty(), @@ -92,7 +93,11 @@ impl ImportDirective { } NameBinding { - kind: NameBindingKind::Import { binding: binding, id: self.id }, + kind: NameBindingKind::Import { + binding: binding, + id: self.id, + privacy_error: privacy_error, + }, span: Some(self.span), modifiers: modifiers, } @@ -220,7 +225,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { span: None, }); let dummy_binding = - self.resolver.new_name_binding(e.import_directive.import(dummy_binding)); + self.resolver.new_name_binding(e.import_directive.import(dummy_binding, None)); let _ = e.source_module.try_define_child(target, ValueNS, dummy_binding); let _ = e.source_module.try_define_child(target, TypeNS, dummy_binding); @@ -296,7 +301,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { &import_directive.module_path, UseLexicalScopeFlag::DontUseLexicalScope, import_directive.span) - .and_then(|(containing_module, lp)| { + .and_then(|containing_module| { // We found the module that the target is contained // within. Attempt to resolve the import within it. if let SingleImport(target, source) = import_directive.subclass { @@ -304,10 +309,9 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { containing_module, target, source, - import_directive, - lp) + import_directive) } else { - self.resolve_glob_import(module_, containing_module, import_directive, lp) + self.resolve_glob_import(module_, containing_module, import_directive) } }) .and_then(|()| { @@ -333,26 +337,14 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { target_module: Module<'b>, target: Name, source: Name, - directive: &ImportDirective, - lp: LastPrivate) + directive: &ImportDirective) -> ResolveResult<()> { - debug!("(resolving single import) resolving `{}` = `{}::{}` from `{}` id {}, last \ - private {:?}", + debug!("(resolving single import) resolving `{}` = `{}::{}` from `{}` id {}", target, module_to_string(&target_module), source, module_to_string(module_), - directive.id, - lp); - - let lp = match lp { - LastMod(lp) => lp, - LastImport {..} => { - self.resolver - .session - .span_bug(directive.span, "not expecting Import here, must be LastMod") - } - }; + directive.id); // If this is a circular import, we temporarily count it as determined so that // it fails (as opposed to being indeterminate) when nothing else can define it. @@ -433,6 +425,8 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { _ => {} } + let mut privacy_error = None; + let mut report_privacy_error = true; for &(ns, result) in &[(ValueNS, &value_result), (TypeNS, &type_result)] { if let Success(binding) = *result { if !binding.defined_with(DefModifiers::IMPORTABLE) { @@ -440,38 +434,34 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { span_err!(self.resolver.session, directive.span, E0253, "{}", &msg); } - self.define(module_, target, ns, directive.import(binding)); + privacy_error = if !self.resolver.is_visible(binding, target_module) { + Some(Box::new(PrivacyError(directive.span, source, binding))) + } else { + report_privacy_error = false; + None + }; + + self.define(module_, target, ns, directive.import(binding, privacy_error.clone())); } } + if report_privacy_error { // then all successful namespaces are privacy errors + // We report here so there is an error even if the imported name is not used + self.resolver.privacy_errors.push(*privacy_error.unwrap()); + } + // 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. module_.decrement_outstanding_references_for(target, ValueNS); module_.decrement_outstanding_references_for(target, TypeNS); - let def_and_priv = |binding: &NameBinding| { - let last_private = - if binding.is_public() { lp } else { DependsOn(binding.local_def_id().unwrap()) }; - (binding.def().unwrap(), last_private) - }; - let value_def_and_priv = value_result.success().map(&def_and_priv); - let type_def_and_priv = type_result.success().map(&def_and_priv); - - let import_lp = LastImport { - value_priv: value_def_and_priv.map(|(_, p)| p), - value_used: Used, - type_priv: type_def_and_priv.map(|(_, p)| p), - type_used: Used, - }; - - let write_path_resolution = |(def, _)| { - let path_resolution = - PathResolution { base_def: def, last_private: import_lp, depth: 0 }; - self.resolver.def_map.borrow_mut().insert(directive.id, path_resolution); + let def = match type_result.success().and_then(NameBinding::def) { + Some(def) => def, + None => value_result.success().and_then(NameBinding::def).unwrap(), }; - value_def_and_priv.map(&write_path_resolution); - type_def_and_priv.map(&write_path_resolution); + let path_resolution = PathResolution { base_def: def, depth: 0 }; + self.resolver.def_map.borrow_mut().insert(directive.id, path_resolution); debug!("(resolving single import) successfully resolved import"); return Success(()); @@ -484,8 +474,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { fn resolve_glob_import(&mut self, module_: Module<'b>, target_module: Module<'b>, - directive: &ImportDirective, - lp: LastPrivate) + directive: &ImportDirective) -> ResolveResult<()> { // We must bail out if the node has unresolved imports of any kind (including globs). if target_module.pub_count.get() > 0 { @@ -503,7 +492,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { build_reduced_graph::populate_module_if_necessary(self.resolver, target_module); target_module.for_each_child(|name, ns, binding| { if !binding.defined_with(DefModifiers::IMPORTABLE | DefModifiers::PUBLIC) { return } - self.define(module_, name, ns, directive.import(binding)); + self.define(module_, name, ns, directive.import(binding, None)); if ns == TypeNS && directive.is_public && binding.defined_with(DefModifiers::PRIVATE_VARIANT) { @@ -521,7 +510,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { self.resolver.def_map.borrow_mut().insert(directive.id, PathResolution { base_def: Def::Mod(did), - last_private: lp, depth: 0, }); } diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 4061d3a2028..8db04c2da20 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -54,7 +54,6 @@ use middle::const_eval::EvalHint::UncheckedExprHint; use middle::def::{self, Def}; use middle::def_id::DefId; use middle::resolve_lifetime as rl; -use middle::privacy::{AllPublic, LastMod}; use middle::subst::{FnSpace, TypeSpace, SelfSpace, Subst, Substs, ParamSpace}; use middle::traits; use middle::ty::{self, Ty, ToPredicate, TypeFoldable}; @@ -1650,7 +1649,6 @@ pub fn ast_ty_to_ty<'tcx>(this: &AstConv<'tcx>, // Create some fake resolution that can't possibly be a type. def::PathResolution { base_def: Def::Mod(tcx.map.local_def_id(ast::CRATE_NODE_ID)), - last_private: LastMod(AllPublic), depth: path.segments.len() } } else { @@ -1674,7 +1672,6 @@ pub fn ast_ty_to_ty<'tcx>(this: &AstConv<'tcx>, // Write back the new resolution. tcx.def_map.borrow_mut().insert(ast_ty.id, def::PathResolution { base_def: def, - last_private: path_res.last_private, depth: 0 }); } diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index bd2c7b39153..f837d354acd 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -12,7 +12,6 @@ use middle::def::{self, Def}; use middle::infer::{self, TypeOrigin}; use middle::pat_util::{PatIdMap, pat_id_map, pat_is_binding}; use middle::pat_util::pat_is_resolved_const; -use middle::privacy::{AllPublic, LastMod}; use middle::subst::Substs; use middle::ty::{self, Ty, TypeFoldable, LvaluePreference}; use check::{check_expr, check_expr_has_type, check_expr_with_expectation}; @@ -219,7 +218,6 @@ pub fn check_pat<'a, 'tcx>(pcx: &pat_ctxt<'a, 'tcx>, let sentinel = fcx.tcx().map.local_def_id(ast::CRATE_NODE_ID); def::PathResolution { base_def: Def::Mod(sentinel), - last_private: LastMod(AllPublic), depth: path.segments.len() } } else { diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index fc2dd4475e3..31e23344734 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -14,7 +14,6 @@ use astconv::AstConv; use check::FnCtxt; use middle::def::Def; use middle::def_id::DefId; -use middle::privacy::{AllPublic, DependsOn, LastPrivate, LastMod}; use middle::subst; use middle::traits; use middle::ty::{self, ToPredicate, ToPolyTraitRef, TraitRef, TypeFoldable}; @@ -334,28 +333,21 @@ pub fn resolve_ufcs<'a, 'tcx>(fcx: &FnCtxt<'a, 'tcx>, method_name: ast::Name, self_ty: ty::Ty<'tcx>, expr_id: ast::NodeId) - -> Result<(Def, LastPrivate), MethodError<'tcx>> + -> Result<Def, MethodError<'tcx>> { let mode = probe::Mode::Path; let pick = try!(probe::probe(fcx, span, mode, method_name, self_ty, expr_id)); - let def_id = pick.item.def_id(); - let mut lp = LastMod(AllPublic); + let def = pick.item.def(); + if let probe::InherentImplPick = pick.kind { - if pick.item.vis() != hir::Public { - lp = LastMod(DependsOn(def_id)); + if pick.item.vis() != hir::Public && !fcx.private_item_is_visible(def.def_id()) { + let msg = format!("{} `{}` is private", def.kind_name(), &method_name.as_str()); + fcx.tcx().sess.span_err(span, &msg); } } - let def_result = match pick.item { - ty::ImplOrTraitItem::MethodTraitItem(..) => Def::Method(def_id), - ty::ImplOrTraitItem::ConstTraitItem(..) => Def::AssociatedConst(def_id), - ty::ImplOrTraitItem::TypeTraitItem(..) => { - fcx.tcx().sess.span_bug(span, "resolve_ufcs: probe picked associated type"); - } - }; - Ok((def_result, lp)) + Ok(def) } - /// Find item with name `item_name` defined in `trait_def_id` /// and return it, or `None`, if no such item. fn trait_item<'tcx>(tcx: &ty::ctxt<'tcx>, diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 7b2e678499b..46145b2a0a4 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -91,7 +91,6 @@ use middle::def_id::DefId; use middle::infer; use middle::infer::{TypeOrigin, type_variable}; use middle::pat_util::{self, pat_id_map}; -use middle::privacy::{AllPublic, LastMod}; use middle::subst::{self, Subst, Substs, VecPerParamSpace, ParamSpace}; use middle::traits::{self, report_fulfillment_errors}; use middle::ty::{GenericPredicates, TypeScheme}; @@ -2013,6 +2012,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Err(errors) => { report_fulfillment_errors(self.infcx(), &errors); } } } + + fn private_item_is_visible(&self, def_id: DefId) -> bool { + match self.tcx().map.as_local_node_id(def_id) { + Some(node_id) => self.tcx().map.private_item_is_visible_from(node_id, self.body_id), + None => false, // Private items from other crates are never visible + } + } } impl<'a, 'tcx> RegionScope for FnCtxt<'a, 'tcx> { @@ -3348,7 +3354,6 @@ fn check_expr_with_unifier<'a, 'tcx, F>(fcx: &FnCtxt<'a, 'tcx>, // Create some fake resolution that can't possibly be a type. def::PathResolution { base_def: Def::Mod(tcx.map.local_def_id(ast::CRATE_NODE_ID)), - last_private: LastMod(AllPublic), depth: path.segments.len() } } else { @@ -3787,12 +3792,11 @@ pub fn resolve_ty_and_def_ufcs<'a, 'b, 'tcx>(fcx: &FnCtxt<'b, 'tcx>, let item_segment = path.segments.last().unwrap(); let item_name = item_segment.identifier.name; match method::resolve_ufcs(fcx, span, item_name, ty, node_id) { - Ok((def, lp)) => { + Ok(def) => { // Write back the new resolution. fcx.ccx.tcx.def_map.borrow_mut() .insert(node_id, def::PathResolution { base_def: def, - last_private: path_res.last_private.or(lp), depth: 0 }); Some((Some(ty), slice::ref_slice(item_segment), def)) diff --git a/src/test/auxiliary/ambig_impl_2_lib.rs b/src/test/auxiliary/ambig_impl_2_lib.rs index bd23fb88217..4ba0ccdba9b 100644 --- a/src/test/auxiliary/ambig_impl_2_lib.rs +++ b/src/test/auxiliary/ambig_impl_2_lib.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -trait me { +pub trait me { fn me(&self) -> usize; } impl me for usize { fn me(&self) -> usize { *self } } diff --git a/src/test/auxiliary/struct_field_privacy.rs b/src/test/auxiliary/struct_field_privacy.rs index fe1dc9d1c8c..5fea97da03e 100644 --- a/src/test/auxiliary/struct_field_privacy.rs +++ b/src/test/auxiliary/struct_field_privacy.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -struct A { +pub struct A { a: isize, pub b: isize, } diff --git a/src/test/compile-fail/blind-item-block-middle.rs b/src/test/compile-fail/blind-item-block-middle.rs index 24a1e4e24d8..930f769771d 100644 --- a/src/test/compile-fail/blind-item-block-middle.rs +++ b/src/test/compile-fail/blind-item-block-middle.rs @@ -8,7 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -mod foo { struct bar; } +mod foo { pub struct bar; } fn main() { let bar = 5; diff --git a/src/test/compile-fail/const-pattern-irrefutable.rs b/src/test/compile-fail/const-pattern-irrefutable.rs index 2d345d9142b..bc395af9622 100644 --- a/src/test/compile-fail/const-pattern-irrefutable.rs +++ b/src/test/compile-fail/const-pattern-irrefutable.rs @@ -9,8 +9,8 @@ // except according to those terms. mod foo { - const b: u8 = 2; //~ NOTE constant defined here - const d: u8 = 2; //~ NOTE constant defined here + pub const b: u8 = 2; //~ NOTE constant defined here + pub const d: u8 = 2; //~ NOTE constant defined here } use foo::b as c; //~ NOTE constant imported here diff --git a/src/test/compile-fail/double-import.rs b/src/test/compile-fail/double-import.rs index bf4dc894154..7b915647884 100644 --- a/src/test/compile-fail/double-import.rs +++ b/src/test/compile-fail/double-import.rs @@ -14,11 +14,11 @@ // when reporting the error. mod sub1 { - fn foo() {} // implementation 1 + pub fn foo() {} // implementation 1 } mod sub2 { - fn foo() {} // implementation 2 + pub fn foo() {} // implementation 2 } use sub1::foo; //~ NOTE previous import of `foo` here diff --git a/src/test/compile-fail/export-tag-variant.rs b/src/test/compile-fail/export-tag-variant.rs index 46d872495a6..b6e8cf71ddd 100644 --- a/src/test/compile-fail/export-tag-variant.rs +++ b/src/test/compile-fail/export-tag-variant.rs @@ -14,4 +14,4 @@ mod foo { enum y { y1, } } -fn main() { let z = foo::y::y1; } //~ ERROR: is inaccessible +fn main() { let z = foo::y::y1; } //~ ERROR: enum `y` is private diff --git a/src/test/compile-fail/issue-11680.rs b/src/test/compile-fail/issue-11680.rs index 1bd7b0aa1c2..7dccd781106 100644 --- a/src/test/compile-fail/issue-11680.rs +++ b/src/test/compile-fail/issue-11680.rs @@ -14,8 +14,8 @@ extern crate issue_11680 as other; fn main() { let _b = other::Foo::Bar(1); - //~^ ERROR: variant `Bar` is private + //~^ ERROR: enum `Foo` is private let _b = other::test::Foo::Bar(1); - //~^ ERROR: variant `Bar` is private + //~^ ERROR: enum `Foo` is private } diff --git a/src/test/compile-fail/issue-13407.rs b/src/test/compile-fail/issue-13407.rs index 311280bd497..afb2e867f45 100644 --- a/src/test/compile-fail/issue-13407.rs +++ b/src/test/compile-fail/issue-13407.rs @@ -16,4 +16,5 @@ fn main() { A::C = 1; //~^ ERROR: invalid left-hand side expression //~| ERROR: mismatched types + //~| ERROR: struct `C` is private } diff --git a/src/test/compile-fail/issue-13641.rs b/src/test/compile-fail/issue-13641.rs index 51b6dc0d078..3b690e08f61 100644 --- a/src/test/compile-fail/issue-13641.rs +++ b/src/test/compile-fail/issue-13641.rs @@ -17,9 +17,7 @@ mod a { fn main() { a::Foo::new(); - //~^ ERROR: method `new` is inaccessible - //~^^ NOTE: struct `Foo` is private + //~^ ERROR: struct `Foo` is private a::Bar::new(); - //~^ ERROR: method `new` is inaccessible - //~^^ NOTE: enum `Bar` is private + //~^ ERROR: enum `Bar` is private } diff --git a/src/test/compile-fail/issue-16538.rs b/src/test/compile-fail/issue-16538.rs index 6f627bfe704..205d3251cc2 100644 --- a/src/test/compile-fail/issue-16538.rs +++ b/src/test/compile-fail/issue-16538.rs @@ -9,11 +9,11 @@ // except according to those terms. mod Y { - type X = usize; + pub type X = usize; extern { - static x: *const usize; + pub static x: *const usize; } - fn foo(value: *const X) -> *const X { + pub fn foo(value: *const X) -> *const X { value } } diff --git a/src/test/compile-fail/issue-21221-2.rs b/src/test/compile-fail/issue-21221-2.rs index 4145d20dea5..8c2c27694d9 100644 --- a/src/test/compile-fail/issue-21221-2.rs +++ b/src/test/compile-fail/issue-21221-2.rs @@ -13,7 +13,7 @@ pub mod foo { // note: trait T is not public, but being in the current // crate, it's fine to show it, since the programmer can // decide to make it public based on the suggestion ... - trait T {} + pub trait T {} } // imports should be ignored: use self::bar::T; diff --git a/src/test/compile-fail/issue-25396.rs b/src/test/compile-fail/issue-25396.rs index 3ada57c9993..ec77e6ebd7c 100644 --- a/src/test/compile-fail/issue-25396.rs +++ b/src/test/compile-fail/issue-25396.rs @@ -32,6 +32,6 @@ mod foo { mod bar { pub mod baz {} pub type Quux = i32; - struct blah { x: i8 } + pub struct blah { x: i8 } pub const WOMP: i8 = -5; } diff --git a/src/test/compile-fail/issue-29161.rs b/src/test/compile-fail/issue-29161.rs index 1821f5717cf..f7453c45be6 100644 --- a/src/test/compile-fail/issue-29161.rs +++ b/src/test/compile-fail/issue-29161.rs @@ -13,7 +13,6 @@ mod a { impl Default for A { pub fn default() -> A { - //~^ ERROR E0449 A; } } @@ -22,5 +21,5 @@ mod a { fn main() { a::A::default(); - //~^ ERROR method `default` is inaccessible + //~^ ERROR struct `A` is private } diff --git a/src/test/compile-fail/privacy-in-paths.rs b/src/test/compile-fail/privacy-in-paths.rs new file mode 100644 index 00000000000..0a8689ea6d6 --- /dev/null +++ b/src/test/compile-fail/privacy-in-paths.rs @@ -0,0 +1,40 @@ +// Copyright 2016 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 <LICENSE-APACHE or +// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license +// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +mod foo { + pub use self::bar::S; + mod bar { + pub struct S; + pub use baz; + } + + trait T { + type Assoc; + } + impl T for () { + type Assoc = S; + } +} + +impl foo::S { + fn f() {} +} + +pub mod baz { + fn f() {} + + fn g() { + ::foo::bar::baz::f(); //~ERROR module `bar` is private + ::foo::bar::S::f(); //~ERROR module `bar` is private + <() as ::foo::T>::Assoc::f(); //~ERROR trait `T` is private + } +} + +fn main() {} diff --git a/src/test/compile-fail/privacy-ns2.rs b/src/test/compile-fail/privacy-ns2.rs index 7fe0574ab7d..bf296220d2a 100644 --- a/src/test/compile-fail/privacy-ns2.rs +++ b/src/test/compile-fail/privacy-ns2.rs @@ -25,14 +25,13 @@ pub mod foo1 { } fn test_single1() { - // In an ideal world, these would be private instead of inaccessible. - use foo1::Bar; //~ ERROR `Bar` is inaccessible + use foo1::Bar; //~ ERROR function `Bar` is private Bar(); } fn test_list1() { - use foo1::{Bar,Baz}; //~ ERROR `Bar` is inaccessible + use foo1::{Bar,Baz}; //~ ERROR `Bar` is private Bar(); } @@ -47,7 +46,7 @@ pub mod foo2 { } fn test_single2() { - use foo2::Bar; //~ ERROR `Bar` is private + use foo2::Bar; //~ ERROR trait `Bar` is private let _x : Box<Bar>; } diff --git a/src/test/compile-fail/privacy-ufcs.rs b/src/test/compile-fail/privacy-ufcs.rs index ccb379c7179..28c1a003e39 100644 --- a/src/test/compile-fail/privacy-ufcs.rs +++ b/src/test/compile-fail/privacy-ufcs.rs @@ -19,6 +19,5 @@ mod foo { } fn main() { - <i32 as ::foo::Bar>::baz(); //~ERROR method `baz` is inaccessible - //~^NOTE: trait `Bar` is private + <i32 as ::foo::Bar>::baz(); //~ERROR trait `Bar` is private } diff --git a/src/test/compile-fail/privacy1.rs b/src/test/compile-fail/privacy1.rs index 495cdc3fe62..9b11eafaa63 100644 --- a/src/test/compile-fail/privacy1.rs +++ b/src/test/compile-fail/privacy1.rs @@ -72,7 +72,6 @@ mod bar { 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(); @@ -91,7 +90,6 @@ fn lol() { bar::A::foo(); bar::A::bar(); //~ ERROR: method `bar` is private bar::A.foo2(); - bar::A.bar2(); //~ ERROR: method `bar2` is private } mod foo { @@ -99,19 +97,14 @@ mod foo { ::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 inaccessible - //~^ NOTE: module `baz` is private - ::bar::baz::A::bar(); //~ ERROR: method `bar` is private - ::bar::baz::A.foo2(); //~ ERROR: struct `A` is inaccessible - //~^ NOTE: module `baz` is private - ::bar::baz::A.bar2(); //~ ERROR: struct `A` is inaccessible - //~^ ERROR: method `bar2` is private - //~^^ NOTE: module `baz` is private + ::bar::baz::A::foo(); //~ ERROR: module `baz` is private + ::bar::baz::A::bar(); //~ ERROR: module `baz` is private + //~^ ERROR: method `bar` is private + ::bar::baz::A.foo2(); //~ ERROR: module `baz` is private + ::bar::baz::A.bar2(); //~ ERROR: module `baz` is private let _: isize = - ::bar::B::foo(); //~ ERROR: method `foo` is inaccessible - //~^ NOTE: trait `B` is private + ::bar::B::foo(); //~ ERROR: trait `B` is private ::lol(); ::bar::Enum::Pub; @@ -126,19 +119,14 @@ mod foo { ::bar::gpub(); - ::bar::baz::foo(); //~ ERROR: function `foo` is inaccessible - //~^ NOTE: module `baz` is private - ::bar::baz::bar(); //~ ERROR: function `bar` is inaccessible - //~^ NOTE: module `baz` is private + ::bar::baz::foo(); //~ ERROR: module `baz` is private + ::bar::baz::bar(); //~ ERROR: module `baz` is private } fn test2() { use bar::baz::{foo, bar}; - //~^ ERROR: function `foo` is inaccessible - //~| NOTE: module `baz` is private - //~| ERROR: function `bar` is inaccessible - //~| NOTE: module `baz` is private - + //~^ ERROR: module `baz` is private + //~| ERROR: module `baz` is private foo(); bar(); @@ -169,8 +157,7 @@ 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::i::A; //~ ERROR: struct `A` is inaccessible - //~^ NOTE: module `i` is private + use self::foo::i::A; //~ ERROR: module `i` is private pub mod foo { pub use self::i::A as foo; diff --git a/src/test/compile-fail/privacy2.rs b/src/test/compile-fail/privacy2.rs index fd8f8d20b7b..abf702204d1 100644 --- a/src/test/compile-fail/privacy2.rs +++ b/src/test/compile-fail/privacy2.rs @@ -16,7 +16,7 @@ mod bar { pub use self::glob::*; - mod glob { + pub mod glob { use foo; } } diff --git a/src/test/compile-fail/privacy4.rs b/src/test/compile-fail/privacy4.rs index 8e9998dd597..d9f76744284 100644 --- a/src/test/compile-fail/privacy4.rs +++ b/src/test/compile-fail/privacy4.rs @@ -28,7 +28,7 @@ mod bar { pub fn foo() {} fn test2() { - use bar::glob::gpriv; //~ ERROR: function `gpriv` is private + use bar::glob::gpriv; //~ ERROR: module `glob` is private gpriv(); } diff --git a/src/test/compile-fail/private-impl-method.rs b/src/test/compile-fail/private-impl-method.rs index c6e329aab04..e04380f12ac 100644 --- a/src/test/compile-fail/private-impl-method.rs +++ b/src/test/compile-fail/private-impl-method.rs @@ -18,7 +18,14 @@ mod a { } } +fn f() { + impl a::Foo { + fn bar(&self) {} // This should be visible outside `f` + } +} + fn main() { let s = a::Foo { x: 1 }; + s.bar(); s.foo(); //~ ERROR method `foo` is private } diff --git a/src/test/compile-fail/struct-field-privacy.rs b/src/test/compile-fail/struct-field-privacy.rs index 2ff48b73e29..1dd8ec0136e 100644 --- a/src/test/compile-fail/struct-field-privacy.rs +++ b/src/test/compile-fail/struct-field-privacy.rs @@ -17,7 +17,7 @@ struct A { } mod inner { - struct A { + pub struct A { a: isize, pub b: isize, } @@ -28,9 +28,6 @@ mod inner { } fn test(a: A, b: inner::A, c: inner::B, d: xc::A, e: xc::B) { - //~^ ERROR: struct `A` is private - //~^^ ERROR: struct `A` is private - a.a; b.a; //~ ERROR: field `a` of struct `inner::A` is private b.b; diff --git a/src/test/compile-fail/struct-variant-privacy-xc.rs b/src/test/compile-fail/struct-variant-privacy-xc.rs index b8be7d0cdc2..8507acd26ce 100644 --- a/src/test/compile-fail/struct-variant-privacy-xc.rs +++ b/src/test/compile-fail/struct-variant-privacy-xc.rs @@ -13,7 +13,7 @@ extern crate struct_variant_privacy; fn f(b: struct_variant_privacy::Bar) { //~ ERROR enum `Bar` is private match b { - struct_variant_privacy::Bar::Baz { a: _a } => {} //~ ERROR variant `Baz` is private + struct_variant_privacy::Bar::Baz { a: _a } => {} //~ ERROR enum `Bar` is private } } diff --git a/src/test/compile-fail/struct-variant-privacy.rs b/src/test/compile-fail/struct-variant-privacy.rs index f36862364e7..7de4ca62555 100644 --- a/src/test/compile-fail/struct-variant-privacy.rs +++ b/src/test/compile-fail/struct-variant-privacy.rs @@ -15,8 +15,7 @@ mod foo { fn f(b: foo::Bar) { //~ ERROR enum `Bar` is private match b { - foo::Bar::Baz { a: _a } => {} //~ ERROR variant `Baz` is inaccessible - // ^~ ERROR enum `Bar` is private + foo::Bar::Baz { a: _a } => {} //~ ERROR enum `Bar` is private } } diff --git a/src/test/compile-fail/task-rng-isnt-sendable.rs b/src/test/compile-fail/task-rng-isnt-sendable.rs index dc3385f4bb9..65801a5704b 100644 --- a/src/test/compile-fail/task-rng-isnt-sendable.rs +++ b/src/test/compile-fail/task-rng-isnt-sendable.rs @@ -10,7 +10,7 @@ // ensure that the ThreadRng isn't/doesn't become accidentally sendable. -use std::rand; +use std::rand; //~ ERROR: module `rand` is private fn test_send<S: Send>() {} diff --git a/src/test/compile-fail/use-mod-3.rs b/src/test/compile-fail/use-mod-3.rs index bd954272fcc..cce500800ca 100644 --- a/src/test/compile-fail/use-mod-3.rs +++ b/src/test/compile-fail/use-mod-3.rs @@ -12,8 +12,7 @@ use foo::bar::{ self //~ ERROR module `bar` is private }; use foo::bar::{ - Bar //~ ERROR type `Bar` is inaccessible - //~^ NOTE module `bar` is private + Bar //~ ERROR module `bar` is private }; mod foo { diff --git a/src/test/compile-fail/xcrate-private-by-default.rs b/src/test/compile-fail/xcrate-private-by-default.rs index 43be96965d0..3bd4c780625 100644 --- a/src/test/compile-fail/xcrate-private-by-default.rs +++ b/src/test/compile-fail/xcrate-private-by-default.rs @@ -43,13 +43,13 @@ fn main() { // public items in a private mod should be inaccessible static_priv_by_default::foo::a; - //~^ ERROR: static `a` is private + //~^ ERROR: module `foo` is private static_priv_by_default::foo::b; - //~^ ERROR: function `b` is private + //~^ ERROR: module `foo` is private static_priv_by_default::foo::c; - //~^ ERROR: struct `c` is private + //~^ ERROR: module `foo` is private foo::<static_priv_by_default::foo::d>(); - //~^ ERROR: enum `d` is private + //~^ ERROR: module `foo` is private foo::<static_priv_by_default::foo::e>(); - //~^ ERROR: type `e` is private + //~^ ERROR: module `foo` is private } |