bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

resolve: refactor away PRIVATE_VARIANT and ensure that restricted

Browse source 
reexports of private variants are handled correctly.
This commit is contained in:
Jeffrey Seyfried 2016-04-11 05:35:18 +00:00
parent a0c3ce3424
commit 0833a89a3e
3 changed files with 32 additions and 30 deletions
Download patch file Download diff file Expand all files Collapse all files

17
src/librustc_resolve/build_reduced_graph.rs
View file

@ -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.

25
src/librustc_resolve/lib.rs
View file

@ -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<Span>,
// 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));

20
src/librustc_resolve/resolve_imports.rs
View file

@ -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::<Vec<_>>();
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);