From a4196cd490deba9f5d885f6c9f335c05f1048c24 Mon Sep 17 00:00:00 2001
From: Jeffrey Seyfried <jeffrey.seyfried@gmail.com>
Date: Sat, 9 Apr 2016 23:19:53 +0000
Subject: [PATCH] resolve: Use `vis: ty::Visibility` instead of `is_public:
 bool`

---
 src/librustc_resolve/build_reduced_graph.rs | 115 +++++++++-----------
 src/librustc_resolve/lib.rs                 |  91 +++++++++++-----
 src/librustc_resolve/resolve_imports.rs     |  26 ++---
 3 files changed, 127 insertions(+), 105 deletions(-)

diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index 003450cd6fd..842bf20672a 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -53,10 +53,10 @@ impl<'a> ToNameBinding<'a> for (Module<'a>, Span) {
     }
 }
 
-impl<'a> ToNameBinding<'a> for (Def, Span, DefModifiers) {
+impl<'a> ToNameBinding<'a> for (Def, Span, DefModifiers, ty::Visibility) {
     fn to_name_binding(self) -> NameBinding<'a> {
         let kind = NameBindingKind::Def(self.0);
-        NameBinding { modifiers: self.2, kind: kind, span: Some(self.1) }
+        NameBinding { modifiers: self.2, kind: kind, span: Some(self.1), vis: self.3 }
     }
 }
 
@@ -105,12 +105,9 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
         let parent = *parent_ref;
         let name = item.name;
         let sp = item.span;
-        let is_public = item.vis == hir::Public;
-        let modifiers = if is_public {
-            DefModifiers::PUBLIC
-        } else {
-            DefModifiers::empty()
-        } | DefModifiers::IMPORTABLE;
+        let modifiers = DefModifiers::IMPORTABLE;
+        self.current_module = parent;
+        let vis = self.resolve_visibility(&item.vis);
 
         match item.node {
             ItemUse(ref view_path) => {
@@ -172,7 +169,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
                                                     subclass,
                                                     view_path.span,
                                                     item.id,
-                                                    is_public,
+                                                    vis,
                                                     is_prelude);
                     }
                     ViewPathList(_, ref source_items) => {
@@ -223,7 +220,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
                                                         subclass,
                                                         source_item.span,
                                                         source_item.node.id(),
-                                                        is_public,
+                                                        vis,
                                                         is_prelude);
                         }
                     }
@@ -233,7 +230,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
                                                     GlobImport,
                                                     view_path.span,
                                                     item.id,
-                                                    is_public,
+                                                    vis,
                                                     is_prelude);
                     }
                 }
@@ -249,7 +246,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
                     };
                     let parent_link = ModuleParentLink(parent, name);
                     let def = Def::Mod(def_id);
-                    let module = self.new_extern_crate_module(parent_link, def, is_public, item.id);
+                    let module = self.new_extern_crate_module(parent_link, def, vis, item.id);
                     self.define(parent, name, TypeNS, (module, sp));
 
                     self.build_reduced_graph_for_external_crate(module);
@@ -259,7 +256,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
             ItemMod(..) => {
                 let parent_link = ModuleParentLink(parent, name);
                 let def = Def::Mod(self.ast_map.local_def_id(item.id));
-                let module = self.new_module(parent_link, Some(def), false, is_public);
+                let module = self.new_module(parent_link, Some(def), false, vis);
                 self.define(parent, name, TypeNS, (module, sp));
                 parent.module_children.borrow_mut().insert(item.id, module);
                 *parent_ref = module;
@@ -271,33 +268,32 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
             ItemStatic(_, m, _) => {
                 let mutbl = m == hir::MutMutable;
                 let def = Def::Static(self.ast_map.local_def_id(item.id), mutbl);
-                self.define(parent, name, ValueNS, (def, sp, modifiers));
+                self.define(parent, name, ValueNS, (def, sp, modifiers, vis));
             }
             ItemConst(_, _) => {
                 let def = Def::Const(self.ast_map.local_def_id(item.id));
-                self.define(parent, name, ValueNS, (def, sp, modifiers));
+                self.define(parent, name, ValueNS, (def, sp, modifiers, vis));
             }
             ItemFn(_, _, _, _, _, _) => {
                 let def = Def::Fn(self.ast_map.local_def_id(item.id));
-                self.define(parent, name, ValueNS, (def, sp, modifiers));
+                self.define(parent, name, ValueNS, (def, sp, modifiers, vis));
             }
 
             // These items live in the type namespace.
             ItemTy(..) => {
                 let def = Def::TyAlias(self.ast_map.local_def_id(item.id));
-                self.define(parent, name, TypeNS, (def, sp, modifiers));
+                self.define(parent, name, TypeNS, (def, sp, modifiers, vis));
             }
 
             ItemEnum(ref enum_definition, _) => {
                 let parent_link = ModuleParentLink(parent, name);
                 let def = Def::Enum(self.ast_map.local_def_id(item.id));
-                let module = self.new_module(parent_link, Some(def), false, is_public);
+                let module = self.new_module(parent_link, Some(def), false, vis);
                 self.define(parent, name, TypeNS, (module, sp));
 
-                let variant_modifiers = if is_public {
-                    DefModifiers::empty()
-                } else {
-                    DefModifiers::PRIVATE_VARIANT
+                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);
@@ -310,20 +306,20 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
             ItemStruct(ref struct_def, _) => {
                 // Define a name in the type namespace.
                 let def = Def::Struct(self.ast_map.local_def_id(item.id));
-                self.define(parent, name, TypeNS, (def, sp, modifiers));
+                self.define(parent, name, TypeNS, (def, sp, modifiers, vis));
 
                 // If this is a newtype or unit-like struct, define a name
                 // in the value namespace as well
                 if !struct_def.is_struct() {
                     let def = Def::Struct(self.ast_map.local_def_id(struct_def.id()));
-                    self.define(parent, name, ValueNS, (def, sp, modifiers));
+                    self.define(parent, name, ValueNS, (def, sp, modifiers, vis));
                 }
 
                 // Record the def ID and fields of this struct.
-                let field_names = struct_def.fields()
-                                            .iter()
-                                            .map(|f| f.name)
-                                            .collect();
+                let field_names = struct_def.fields().iter().map(|field| {
+                    self.resolve_visibility(&field.vis);
+                    field.name
+                }).collect();
                 let item_def_id = self.ast_map.local_def_id(item.id);
                 self.structs.insert(item_def_id, field_names);
             }
@@ -336,7 +332,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
                 // Add all the items within to a new module.
                 let parent_link = ModuleParentLink(parent, name);
                 let def = Def::Trait(def_id);
-                let module_parent = self.new_module(parent_link, Some(def), false, is_public);
+                let module_parent = self.new_module(parent_link, Some(def), false, vis);
                 self.define(parent, name, TypeNS, (module_parent, sp));
 
                 // Add the names of all the items to the trait info.
@@ -348,8 +344,8 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
                         hir::TypeTraitItem(..) => (Def::AssociatedTy(def_id, item_def_id), TypeNS),
                     };
 
-                    let modifiers = DefModifiers::PUBLIC; // NB: not DefModifiers::IMPORTABLE
-                    self.define(module_parent, item.name, ns, (def, item.span, modifiers));
+                    let modifiers = DefModifiers::empty(); // NB: not DefModifiers::IMPORTABLE
+                    self.define(module_parent, item.name, ns, (def, item.span, modifiers, vis));
 
                     self.trait_item_map.insert((item.name, def_id), item_def_id);
                 }
@@ -373,11 +369,12 @@ 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::PUBLIC | DefModifiers::IMPORTABLE | variant_modifiers;
+        let modifiers = DefModifiers::IMPORTABLE | variant_modifiers;
         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));
-        self.define(parent, name, TypeNS, (def, variant.span, modifiers));
+        self.define(parent, name, ValueNS, (def, variant.span, modifiers, vis));
+        self.define(parent, name, TypeNS, (def, variant.span, modifiers, vis));
     }
 
     /// Constructs the reduced graph for one foreign item.
@@ -385,12 +382,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
                                             foreign_item: &ForeignItem,
                                             parent: Module<'b>) {
         let name = foreign_item.name;
-        let is_public = foreign_item.vis == hir::Public;
-        let modifiers = if is_public {
-            DefModifiers::PUBLIC
-        } else {
-            DefModifiers::empty()
-        } | DefModifiers::IMPORTABLE;
+        let modifiers = DefModifiers::IMPORTABLE;
 
         let def = match foreign_item.node {
             ForeignItemFn(..) => {
@@ -400,7 +392,9 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
                 Def::Static(self.ast_map.local_def_id(foreign_item.id), m)
             }
         };
-        self.define(parent, name, ValueNS, (def, foreign_item.span, modifiers));
+        self.current_module = parent;
+        let vis = self.resolve_visibility(&foreign_item.vis);
+        self.define(parent, name, ValueNS, (def, foreign_item.span, modifiers, vis));
     }
 
     fn build_reduced_graph_for_block(&mut self, block: &Block, parent: &mut Module<'b>) {
@@ -412,7 +406,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
                    block_id);
 
             let parent_link = BlockParentLink(parent, block_id);
-            let new_module = self.new_module(parent_link, None, false, false);
+            let new_module = self.new_module(parent_link, None, false, parent.vis);
             parent.module_children.borrow_mut().insert(block_id, new_module);
             *parent = new_module;
         }
@@ -434,32 +428,27 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
         }
 
         let name = xcdef.name;
-        let is_public = xcdef.vis == ty::Visibility::Public || parent.is_trait();
-
-        let mut modifiers = DefModifiers::empty();
-        if is_public {
-            modifiers = modifiers | DefModifiers::PUBLIC;
-        }
-        if parent.is_normal() {
-            modifiers = modifiers | DefModifiers::IMPORTABLE;
-        }
+        let vis = if parent.is_trait() { ty::Visibility::Public } else { xcdef.vis };
+        let modifiers = match parent.is_normal() {
+            true => DefModifiers::IMPORTABLE,
+            false => DefModifiers::empty(),
+        };
 
         match def {
             Def::Mod(_) | Def::ForeignMod(_) | Def::Enum(..) => {
-                debug!("(building reduced graph for external crate) building module {} {}",
-                       name,
-                       is_public);
+                debug!("(building reduced graph for external crate) building module {} {:?}",
+                       name, vis);
                 let parent_link = ModuleParentLink(parent, name);
-                let module = self.new_module(parent_link, Some(def), true, is_public);
+                let module = self.new_module(parent_link, Some(def), true, vis);
                 self.try_define(parent, name, TypeNS, (module, DUMMY_SP));
             }
             Def::Variant(_, variant_id) => {
                 debug!("(building reduced graph for external crate) building variant {}", name);
                 // 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::PUBLIC | DefModifiers::IMPORTABLE;
-                self.try_define(parent, name, TypeNS, (def, DUMMY_SP, modifiers));
-                self.try_define(parent, name, ValueNS, (def, DUMMY_SP, modifiers));
+                let modifiers = DefModifiers::IMPORTABLE;
+                self.try_define(parent, name, TypeNS, (def, DUMMY_SP, modifiers, vis));
+                self.try_define(parent, name, ValueNS, (def, DUMMY_SP, modifiers, vis));
                 if self.session.cstore.variant_kind(variant_id) == Some(VariantKind::Struct) {
                     // Not adding fields for variants as they are not accessed with a self receiver
                     self.structs.insert(variant_id, Vec::new());
@@ -472,7 +461,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
             Def::Method(..) => {
                 debug!("(building reduced graph for external crate) building value (fn/static) {}",
                        name);
-                self.try_define(parent, name, ValueNS, (def, DUMMY_SP, modifiers));
+                self.try_define(parent, name, ValueNS, (def, DUMMY_SP, modifiers, vis));
             }
             Def::Trait(def_id) => {
                 debug!("(building reduced graph for external crate) building type {}", name);
@@ -493,21 +482,21 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
                 }
 
                 let parent_link = ModuleParentLink(parent, name);
-                let module = self.new_module(parent_link, Some(def), true, is_public);
+                let module = self.new_module(parent_link, Some(def), true, vis);
                 self.try_define(parent, name, TypeNS, (module, DUMMY_SP));
             }
             Def::TyAlias(..) | Def::AssociatedTy(..) => {
                 debug!("(building reduced graph for external crate) building type {}", name);
-                self.try_define(parent, name, TypeNS, (def, DUMMY_SP, modifiers));
+                self.try_define(parent, name, TypeNS, (def, DUMMY_SP, modifiers, vis));
             }
             Def::Struct(def_id)
                 if self.session.cstore.tuple_struct_definition_if_ctor(def_id).is_none() => {
                 debug!("(building reduced graph for external crate) building type and value for {}",
                        name);
-                self.try_define(parent, name, TypeNS, (def, DUMMY_SP, modifiers));
+                self.try_define(parent, name, TypeNS, (def, DUMMY_SP, modifiers, vis));
                 if let Some(ctor_def_id) = self.session.cstore.struct_ctor_def_id(def_id) {
                     let def = Def::Struct(ctor_def_id);
-                    self.try_define(parent, name, ValueNS, (def, DUMMY_SP, modifiers));
+                    self.try_define(parent, name, ValueNS, (def, DUMMY_SP, modifiers, vis));
                 }
 
                 // Record the def ID and fields of this struct.
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index 5f4244caa62..c1d2b0c4b43 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -55,6 +55,7 @@ use rustc::middle::cstore::CrateStore;
 use rustc::hir::def::*;
 use rustc::hir::def_id::DefId;
 use rustc::hir::pat_util::pat_bindings;
+use rustc::ty;
 use rustc::ty::subst::{ParamSpace, FnSpace, TypeSpace};
 use rustc::hir::{Freevar, FreevarMap, TraitMap, GlobMap};
 use rustc::util::nodemap::{NodeMap, FnvHashMap, FnvHashSet};
@@ -820,7 +821,7 @@ enum ParentLink<'a> {
 pub struct ModuleS<'a> {
     parent_link: ParentLink<'a>,
     def: Option<Def>,
-    is_public: bool,
+    vis: ty::Visibility,
 
     // 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).
@@ -864,12 +865,12 @@ impl<'a> ModuleS<'a> {
     fn new(parent_link: ParentLink<'a>,
            def: Option<Def>,
            external: bool,
-           is_public: bool,
+           vis: ty::Visibility,
            arenas: &'a ResolverArenas<'a>) -> Self {
         ModuleS {
             parent_link: parent_link,
             def: def,
-            is_public: is_public,
+            vis: vis,
             extern_crate_id: None,
             resolutions: RefCell::new(HashMap::new()),
             unresolved_imports: RefCell::new(Vec::new()),
@@ -892,9 +893,10 @@ impl<'a> ModuleS<'a> {
         self.def.as_ref().map(Def::def_id)
     }
 
+    // `self` resolves to the first module ancestor that `is_normal`.
     fn is_normal(&self) -> bool {
         match self.def {
-            Some(Def::Mod(_)) | Some(Def::ForeignMod(_)) => true,
+            Some(Def::Mod(_)) => true,
             _ => false,
         }
     }
@@ -918,23 +920,13 @@ impl<'a> ModuleS<'a> {
 
 impl<'a> fmt::Debug for ModuleS<'a> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f,
-               "{:?}, {}",
-               self.def,
-               if self.is_public {
-                   "public"
-               } else {
-                   "private"
-               })
+        write!(f, "{:?}, {:?}", self.def, self.vis)
     }
 }
 
 bitflags! {
     #[derive(Debug)]
     flags DefModifiers: u8 {
-        // Enum variants are always considered `PUBLIC`, this is needed for `use Enum::Variant`
-        // or `use Enum::*` to work on private enums.
-        const PUBLIC     = 1 << 0,
         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`.
@@ -949,6 +941,9 @@ 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,
 }
 
 #[derive(Clone, Debug)]
@@ -968,13 +963,12 @@ 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 {
-            DefModifiers::PUBLIC
-        } else {
-            DefModifiers::empty()
-        } | DefModifiers::IMPORTABLE;
-
-        NameBinding { modifiers: modifiers, kind: NameBindingKind::Module(module), span: span }
+        NameBinding {
+            modifiers: DefModifiers::IMPORTABLE,
+            kind: NameBindingKind::Module(module),
+            span: span,
+            vis: module.vis,
+        }
     }
 
     fn module(&self) -> Option<Module<'a>> {
@@ -998,7 +992,7 @@ impl<'a> NameBinding<'a> {
     }
 
     fn is_public(&self) -> bool {
-        self.defined_with(DefModifiers::PUBLIC)
+        self.vis == ty::Visibility::Public
     }
 
     fn is_extern_crate(&self) -> bool {
@@ -1148,8 +1142,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
            arenas: &'a ResolverArenas<'a>)
            -> Resolver<'a, 'tcx> {
         let root_def_id = ast_map.local_def_id(CRATE_NODE_ID);
+        let vis = ty::Visibility::Public;
         let graph_root =
-            ModuleS::new(NoParentLink, Some(Def::Mod(root_def_id)), false, true, arenas);
+            ModuleS::new(NoParentLink, Some(Def::Mod(root_def_id)), false, vis, arenas);
         let graph_root = arenas.alloc_module(graph_root);
 
         Resolver {
@@ -1209,17 +1204,17 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                   parent_link: ParentLink<'a>,
                   def: Option<Def>,
                   external: bool,
-                  is_public: bool) -> Module<'a> {
-        self.arenas.alloc_module(ModuleS::new(parent_link, def, external, is_public, self.arenas))
+                  vis: ty::Visibility) -> Module<'a> {
+        self.arenas.alloc_module(ModuleS::new(parent_link, def, external, vis, self.arenas))
     }
 
     fn new_extern_crate_module(&self,
                                parent_link: ParentLink<'a>,
                                def: Def,
-                               is_public: bool,
+                               vis: ty::Visibility,
                                local_node_id: NodeId)
                                -> Module<'a> {
-        let mut module = ModuleS::new(parent_link, Some(def), false, is_public, self.arenas);
+        let mut module = ModuleS::new(parent_link, Some(def), false, vis, self.arenas);
         module.extern_crate_id = Some(local_node_id);
         self.arenas.modules.alloc(module)
     }
@@ -1617,7 +1612,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
     fn resolve_crate(&mut self, krate: &hir::Crate) {
         debug!("(resolving crate) starting");
-
+        self.current_module = self.graph_root;
         intravisit::walk_crate(self, krate);
     }
 
@@ -1980,6 +1975,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                 this.with_self_rib(Def::SelfTy(trait_id, Some((item_id, self_type.id))), |this| {
                     this.with_current_self_type(self_type, |this| {
                         for impl_item in impl_items {
+                            this.resolve_visibility(&impl_item.vis);
                             match impl_item.node {
                                 hir::ImplItemKind::Const(..) => {
                                     // If this is a trait impl, ensure the const
@@ -3379,6 +3375,43 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         }
     }
 
+    fn resolve_visibility(&mut self, vis: &hir::Visibility) -> ty::Visibility {
+        let (path, id) = match *vis {
+            hir::Public => return ty::Visibility::Public,
+            hir::Visibility::Crate => return ty::Visibility::Restricted(ast::CRATE_NODE_ID),
+            hir::Visibility::Restricted { ref path, id } => (path, id),
+            hir::Inherited => {
+                let current_module =
+                    self.get_nearest_normal_module_parent_or_self(self.current_module);
+                let id = self.ast_map.as_local_node_id(current_module.def_id().unwrap()).unwrap();
+                return ty::Visibility::Restricted(id);
+            }
+        };
+
+        let segments: Vec<_> = path.segments.iter().map(|seg| seg.identifier.name).collect();
+        let vis = match self.resolve_module_path(&segments, DontUseLexicalScope, path.span) {
+            Success(module) => {
+                let def = module.def.unwrap();
+                let path_resolution = PathResolution { base_def: def, depth: 0 };
+                self.def_map.borrow_mut().insert(id, path_resolution);
+                ty::Visibility::Restricted(self.ast_map.as_local_node_id(def.def_id()).unwrap())
+            }
+            Failed(Some((span, msg))) => {
+                self.session.span_err(span, &format!("failed to resolve module path. {}", msg));
+                ty::Visibility::Public
+            }
+            _ => {
+                self.session.span_err(path.span, "unresolved module path");
+                ty::Visibility::Public
+            }
+        };
+        if !self.is_accessible(vis) {
+            let msg = format!("visibilities can only be restricted to ancestor modules");
+            self.session.span_err(path.span, &msg);
+        }
+        vis
+    }
+
     fn is_visible(&self, binding: &'a NameBinding<'a>, parent: Module<'a>) -> bool {
         binding.is_public() || parent.is_ancestor_of(self.current_module)
     }
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
index ff684d37653..c22004910c6 100644
--- a/src/librustc_resolve/resolve_imports.rs
+++ b/src/librustc_resolve/resolve_imports.rs
@@ -21,6 +21,7 @@ use UseLexicalScopeFlag::DontUseLexicalScope;
 use {names_to_string, module_to_string};
 use {resolve_error, ResolutionError};
 
+use rustc::ty;
 use rustc::lint;
 use rustc::hir::def::*;
 
@@ -63,7 +64,7 @@ pub struct ImportDirective<'a> {
     subclass: ImportDirectiveSubclass,
     span: Span,
     id: NodeId,
-    is_public: bool, // see note in ImportResolutionPerNamespace about how to use this
+    vis: ty::Visibility, // see note in ImportResolutionPerNamespace about how to use this
     is_prelude: bool,
 }
 
@@ -72,10 +73,7 @@ impl<'a> ImportDirective<'a> {
     // this returns the binding for the name this directive defines in that namespace.
     fn import(&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(),
-        };
+        let mut modifiers = DefModifiers::IMPORTABLE;
         if let GlobImport = self.subclass {
             modifiers = modifiers | DefModifiers::GLOB_IMPORTED;
         }
@@ -88,6 +86,7 @@ impl<'a> ImportDirective<'a> {
             },
             span: Some(self.span),
             modifiers: modifiers,
+            vis: self.vis,
         }
     }
 }
@@ -184,7 +183,7 @@ impl<'a> NameResolution<'a> {
                 // If (1) we don't allow private imports, (2) no public single import can define
                 // 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.is_public &&
+                if !allow_private_imports && directive.vis != ty::Visibility::Public &&
                    !self.binding.map(NameBinding::is_public).unwrap_or(false) {
                     return None;
                 }
@@ -250,7 +249,7 @@ impl<'a> ::ModuleS<'a> {
 
         // Check if the globs are determined
         for directive in self.globs.borrow().iter() {
-            if !allow_private_imports && !directive.is_public { continue }
+            if !allow_private_imports && directive.vis != ty::Visibility::Public { continue }
             match directive.target_module.get() {
                 None => return Indeterminate,
                 Some(target_module) => match target_module.resolve_name(name, ns, false) {
@@ -285,7 +284,7 @@ impl<'a> ::ModuleS<'a> {
                                 subclass: ImportDirectiveSubclass,
                                 span: Span,
                                 id: NodeId,
-                                is_public: bool,
+                                vis: ty::Visibility,
                                 is_prelude: bool) {
         let directive = self.arenas.alloc_import_directive(ImportDirective {
             module_path: module_path,
@@ -293,7 +292,7 @@ impl<'a> ::ModuleS<'a> {
             subclass: subclass,
             span: span,
             id: id,
-            is_public: is_public,
+            vis: vis,
             is_prelude: is_prelude,
         });
 
@@ -337,7 +336,7 @@ impl<'a> ::ModuleS<'a> {
     }
 
     fn define_in_glob_importers(&self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>) {
-        if !binding.defined_with(DefModifiers::PUBLIC | DefModifiers::IMPORTABLE) { return }
+        if !binding.defined_with(DefModifiers::IMPORTABLE) || !binding.is_public() { return }
         for &(importer, directive) in self.glob_importers.borrow_mut().iter() {
             let _ = importer.try_define_child(name, ns, directive.import(binding, None));
         }
@@ -413,6 +412,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                 modifiers: DefModifiers::GLOB_IMPORTED,
                 kind: NameBindingKind::Def(Def::Err),
                 span: None,
+                vis: ty::Visibility::Public,
             });
             let dummy_binding = e.import_directive.import(dummy_binding, None);
 
@@ -569,7 +569,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
 
         match (&value_result, &type_result) {
             (&Success(name_binding), _) if !name_binding.is_import() &&
-                                           directive.is_public &&
+                                           directive.vis == ty::Visibility::Public &&
                                            !name_binding.is_public() => {
                 let msg = format!("`{}` is private, and cannot be reexported", source);
                 let note_msg = format!("consider marking `{}` as `pub` in the imported module",
@@ -580,7 +580,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
             }
 
             (_, &Success(name_binding)) if !name_binding.is_import() &&
-                                           directive.is_public &&
+                                           directive.vis == ty::Visibility::Public &&
                                            !name_binding.is_public() => {
                 if name_binding.is_extern_crate() {
                     let msg = format!("extern crate `{}` is private, and cannot be reexported \
@@ -662,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 | DefModifiers::PUBLIC) {
+            if binding.defined_with(DefModifiers::IMPORTABLE) && binding.is_public() {
                 let _ = module_.try_define_child(name, ns, directive.import(binding, None));
             }
         }