diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs index 842bf20672a..2bec7725b76 100644 --- a/src/librustc_resolve/build_reduced_graph.rs +++ b/src/librustc_resolve/build_reduced_graph.rs @@ -291,14 +291,9 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> { let module = self.new_module(parent_link, Some(def), false, vis); self.define(parent, name, TypeNS, (module, sp)); - let variant_modifiers = match vis { - ty::Visibility::Public => DefModifiers::empty(), - _ => DefModifiers::PRIVATE_VARIANT, - }; for variant in &(*enum_definition).variants { let item_def_id = self.ast_map.local_def_id(item.id); - self.build_reduced_graph_for_variant(variant, item_def_id, - module, variant_modifiers); + self.build_reduced_graph_for_variant(variant, item_def_id, module); } } @@ -358,8 +353,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> { fn build_reduced_graph_for_variant(&mut self, variant: &Variant, item_id: DefId, - parent: Module<'b>, - variant_modifiers: DefModifiers) { + parent: Module<'b>) { let name = variant.node.name; if variant.node.data.is_struct() { // Not adding fields for variants as they are not accessed with a self receiver @@ -369,12 +363,11 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> { // Variants are always treated as importable to allow them to be glob used. // All variants are defined in both type and value namespaces as future-proofing. - let modifiers = DefModifiers::IMPORTABLE | variant_modifiers; + let modifiers = DefModifiers::IMPORTABLE; let def = Def::Variant(item_id, self.ast_map.local_def_id(variant.node.data.id())); - let vis = ty::Visibility::Public; - self.define(parent, name, ValueNS, (def, variant.span, modifiers, vis)); - self.define(parent, name, TypeNS, (def, variant.span, modifiers, vis)); + self.define(parent, name, ValueNS, (def, variant.span, modifiers, parent.vis)); + self.define(parent, name, TypeNS, (def, variant.span, modifiers, parent.vis)); } /// Constructs the reduced graph for one foreign item. diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index e161a42032d..2147331d441 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -919,9 +919,6 @@ bitflags! { #[derive(Debug)] flags DefModifiers: u8 { const IMPORTABLE = 1 << 1, - // Variants are considered `PUBLIC`, but some of them live in private enums. - // We need to track them to prohibit reexports like `pub use PrivEnum::Variant`. - const PRIVATE_VARIANT = 1 << 2, const GLOB_IMPORTED = 1 << 3, } } @@ -932,8 +929,6 @@ pub struct NameBinding<'a> { modifiers: DefModifiers, kind: NameBindingKind<'a>, span: Option, - // Enum variants are always considered `PUBLIC`, this is needed for `use Enum::Variant` - // or `use Enum::*` to work on private enums. vis: ty::Visibility, } @@ -982,8 +977,20 @@ impl<'a> NameBinding<'a> { self.modifiers.contains(modifiers) } - fn is_public(&self) -> bool { - self.vis == ty::Visibility::Public + fn is_pseudo_public(&self) -> bool { + self.pseudo_vis() == ty::Visibility::Public + } + + // We sometimes need to treat variants as `pub` for backwards compatibility + fn pseudo_vis(&self) -> ty::Visibility { + if self.is_variant() { ty::Visibility::Public } else { self.vis } + } + + fn is_variant(&self) -> bool { + match self.kind { + NameBindingKind::Def(Def::Variant(..)) => true, + _ => false, + } } fn is_extern_crate(&self) -> bool { @@ -3301,7 +3308,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { // only if both the module is public and the entity is // declared as public (due to pruning, we don't explore // outside crate private modules => no need to check this) - if !in_module_is_extern || name_binding.is_public() { + if !in_module_is_extern || name_binding.vis == ty::Visibility::Public { lookup_results.push(path); } } @@ -3326,7 +3333,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> { _ => bug!(), }; - if !in_module_is_extern || name_binding.is_public() { + if !in_module_is_extern || name_binding.vis == ty::Visibility::Public { // add the module to the lookup let is_extern = in_module_is_extern || name_binding.is_extern_crate(); worklist.push((module, path_segments, is_extern)); diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 31d1c570026..e712dbdcbf7 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -184,7 +184,7 @@ impl<'a> NameResolution<'a> { // the name, and (3) no public glob has defined the name, the resolution depends // on whether more globs can define the name. if !allow_private_imports && directive.vis != ty::Visibility::Public && - !self.binding.map(NameBinding::is_public).unwrap_or(false) { + !self.binding.map(NameBinding::is_pseudo_public).unwrap_or(false) { return None; } @@ -242,7 +242,8 @@ impl<'a> ::ModuleS<'a> { if let Some(result) = resolution.try_result(ns, allow_private_imports) { // If the resolution doesn't depend on glob definability, check privacy and return. return result.and_then(|binding| { - let allowed = allow_private_imports || !binding.is_import() || binding.is_public(); + let allowed = allow_private_imports || !binding.is_import() || + binding.is_pseudo_public(); if allowed { Success(binding) } else { Failed(None) } }); } @@ -336,7 +337,7 @@ impl<'a> ::ModuleS<'a> { } fn define_in_glob_importers(&self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>) { - if !binding.defined_with(DefModifiers::IMPORTABLE) || !binding.is_public() { return } + if !binding.defined_with(DefModifiers::IMPORTABLE) || !binding.is_pseudo_public() { return } for &(importer, directive) in self.glob_importers.borrow_mut().iter() { let _ = importer.try_define_child(name, ns, directive.import(binding, None)); } @@ -569,7 +570,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { let ast_map = self.resolver.ast_map; match (&value_result, &type_result) { - (&Success(binding), _) if !binding.vis.is_at_least(directive.vis, ast_map) && + (&Success(binding), _) if !binding.pseudo_vis().is_at_least(directive.vis, ast_map) && self.resolver.is_accessible(binding.vis) => { let msg = format!("`{}` is private, and cannot be reexported", source); let note_msg = format!("consider marking `{}` as `pub` in the imported module", @@ -579,7 +580,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { .emit(); } - (_, &Success(binding)) if !binding.vis.is_at_least(directive.vis, ast_map) && + (_, &Success(binding)) if !binding.pseudo_vis().is_at_least(directive.vis, ast_map) && self.resolver.is_accessible(binding.vis) => { if binding.is_extern_crate() { let msg = format!("extern crate `{}` is private, and cannot be reexported \ @@ -661,7 +662,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { resolution.borrow().binding().map(|binding| (*name, binding)) }).collect::>(); for ((name, ns), binding) in bindings { - if binding.defined_with(DefModifiers::IMPORTABLE) && binding.is_public() { + if binding.defined_with(DefModifiers::IMPORTABLE) && binding.is_pseudo_public() { let _ = module_.try_define_child(name, ns, directive.import(binding, None)); } } @@ -697,15 +698,16 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> { None => continue, }; - if binding.is_public() && (binding.is_import() || binding.is_extern_crate()) { + if binding.vis == ty::Visibility::Public && + (binding.is_import() || binding.is_extern_crate()) { if let Some(def) = binding.def() { reexports.push(Export { name: name, def_id: def.def_id() }); } } if let NameBindingKind::Import { binding: orig_binding, id, .. } = binding.kind { - if ns == TypeNS && binding.is_public() && - orig_binding.defined_with(DefModifiers::PRIVATE_VARIANT) { + if ns == TypeNS && orig_binding.is_variant() && + !orig_binding.vis.is_at_least(binding.vis, &self.resolver.ast_map) { let msg = format!("variant `{}` is private, and cannot be reexported \ (error E0364), consider declaring its enum as `pub`", name);