Rollup merge of #33045 - jseyfried:no_def_modifiers, r=eddyb

resolve: Refactor away `DefModifiers`

This refactors away `DefModifiers`, which is unneeded now that #32875 has landed.
r? @eddyb

src/librustc_resolve/Cargo.toml

@@ -12,5 +12,4 @@ crate-type = ["dylib"]
 log = { path = "../liblog" }
 syntax = { path = "../libsyntax" }
 rustc = { path = "../librustc" }
-rustc_bitflags = { path = "../librustc_bitflags" }
 arena = { path = "../libarena" }

src/librustc_resolve/build_reduced_graph.rs

@@ -13,7 +13,6 @@
 //! Here we build the "reduced graph": the graph of the module tree without
 //! any imports resolved.
 
-use DefModifiers;
 use resolve_imports::ImportDirectiveSubclass::{self, GlobImport};
 use Module;
 use Namespace::{self, TypeNS, ValueNS};
@@ -53,10 +52,9 @@ impl<'a> ToNameBinding<'a> for (Module<'a>, Span) {
     }
 }
 
-impl<'a> ToNameBinding<'a> for (Def, Span, DefModifiers, ty::Visibility) {
+impl<'a> ToNameBinding<'a> for (Def, Span, ty::Visibility) {
     fn to_name_binding(self) -> NameBinding<'a> {
-        let kind = NameBindingKind::Def(self.0);
+        NameBinding { kind: NameBindingKind::Def(self.0), span: Some(self.1), vis: self.2 }
-        NameBinding { modifiers: self.2, kind: kind, span: Some(self.1), vis: self.3 }
     }
 }
 
@@ -136,7 +134,6 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
         let parent = *parent_ref;
         let name = item.name;
         let sp = item.span;
-        let modifiers = DefModifiers::IMPORTABLE;
         self.current_module = parent;
         let vis = self.resolve_visibility(&item.vis);
 
@@ -284,21 +281,21 @@ 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, vis));
+                self.define(parent, name, ValueNS, (def, sp, vis));
             }
             ItemConst(_, _) => {
                 let def = Def::Const(self.ast_map.local_def_id(item.id));
-                self.define(parent, name, ValueNS, (def, sp, modifiers, vis));
+                self.define(parent, name, ValueNS, (def, sp, vis));
             }
             ItemFn(_, _, _, _, _, _) => {
                 let def = Def::Fn(self.ast_map.local_def_id(item.id));
-                self.define(parent, name, ValueNS, (def, sp, modifiers, vis));
+                self.define(parent, name, ValueNS, (def, sp, 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, vis));
+                self.define(parent, name, TypeNS, (def, sp, vis));
             }
 
             ItemEnum(ref enum_definition, _) => {
@@ -317,13 +314,13 @@ 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, vis));
+                self.define(parent, name, TypeNS, (def, sp, 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, vis));
+                    self.define(parent, name, ValueNS, (def, sp, vis));
                 }
 
                 // Record the def ID and fields of this struct.
@@ -355,8 +352,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
                         hir::TypeTraitItem(..) => (Def::AssociatedTy(def_id, item_def_id), TypeNS),
                     };
 
-                    let modifiers = DefModifiers::empty(); // NB: not DefModifiers::IMPORTABLE
+                    self.define(module_parent, item.name, ns, (def, item.span, vis));
-                    self.define(module_parent, item.name, ns, (def, item.span, modifiers, vis));
 
                     self.trait_item_map.insert((item.name, def_id), item_def_id);
                 }
@@ -379,11 +375,9 @@ 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;
         let def = Def::Variant(item_id, self.ast_map.local_def_id(variant.node.data.id()));
-
+        self.define(parent, name, ValueNS, (def, variant.span, parent.vis));
-        self.define(parent, name, ValueNS, (def, variant.span, modifiers, parent.vis));
+        self.define(parent, name, TypeNS, (def, variant.span, parent.vis));
-        self.define(parent, name, TypeNS, (def, variant.span, modifiers, parent.vis));
     }
 
     /// Constructs the reduced graph for one foreign item.
@@ -391,7 +385,6 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
                                             foreign_item: &ForeignItem,
                                             parent: Module<'b>) {
         let name = foreign_item.name;
-        let modifiers = DefModifiers::IMPORTABLE;
 
         let def = match foreign_item.node {
             ForeignItemFn(..) => {
@@ -403,7 +396,7 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
         };
         self.current_module = parent;
         let vis = self.resolve_visibility(&foreign_item.vis);
-        self.define(parent, name, ValueNS, (def, foreign_item.span, modifiers, vis));
+        self.define(parent, name, ValueNS, (def, foreign_item.span, vis));
     }
 
     fn build_reduced_graph_for_block(&mut self, block: &Block, parent: &mut Module<'b>) {
@@ -438,10 +431,6 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
 
         let name = xcdef.name;
         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(..) => {
@@ -455,9 +444,8 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
                 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::IMPORTABLE;
+                self.try_define(parent, name, TypeNS, (def, DUMMY_SP, vis));
-                self.try_define(parent, name, TypeNS, (def, DUMMY_SP, modifiers, vis));
+                self.try_define(parent, name, ValueNS, (def, DUMMY_SP, 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());
@@ -470,7 +458,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, vis));
+                self.try_define(parent, name, ValueNS, (def, DUMMY_SP, vis));
             }
             Def::Trait(def_id) => {
                 debug!("(building reduced graph for external crate) building type {}", name);
@@ -496,16 +484,16 @@ impl<'b, 'tcx:'b> Resolver<'b, 'tcx> {
             }
             Def::TyAlias(..) | Def::AssociatedTy(..) => {
                 debug!("(building reduced graph for external crate) building type {}", name);
-                self.try_define(parent, name, TypeNS, (def, DUMMY_SP, modifiers, vis));
+                self.try_define(parent, name, TypeNS, (def, DUMMY_SP, 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, vis));
+                self.try_define(parent, name, TypeNS, (def, DUMMY_SP, 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, vis));
+                    self.try_define(parent, name, ValueNS, (def, DUMMY_SP, vis));
                 }
 
                 // Record the def ID and fields of this struct.

src/librustc_resolve/lib.rs

@@ -29,9 +29,6 @@ extern crate log;
 extern crate syntax;
 extern crate arena;
 #[macro_use]
-#[no_link]
-extern crate rustc_bitflags;
-#[macro_use]
 extern crate rustc;
 
 use self::PatternBindingMode::*;
@@ -915,18 +912,9 @@ impl<'a> fmt::Debug for ModuleS<'a> {
     }
 }
 
-bitflags! {
-    #[derive(Debug)]
-    flags DefModifiers: u8 {
-        const IMPORTABLE = 1 << 1,
-        const GLOB_IMPORTED = 1 << 3,
-    }
-}
-
 // Records a possibly-private value, type, or module definition.
 #[derive(Clone, Debug)]
 pub struct NameBinding<'a> {
-    modifiers: DefModifiers,
     kind: NameBindingKind<'a>,
     span: Option<Span>,
     vis: ty::Visibility,
@@ -938,7 +926,7 @@ enum NameBindingKind<'a> {
     Module(Module<'a>),
     Import {
         binding: &'a NameBinding<'a>,
-        id: NodeId,
+        directive: &'a ImportDirective<'a>,
         // Some(error) if using this imported name causes the import to be a privacy error
         privacy_error: Option<Box<PrivacyError<'a>>>,
     },
@@ -950,7 +938,6 @@ struct PrivacyError<'a>(Span, Name, &'a NameBinding<'a>);
 impl<'a> NameBinding<'a> {
     fn create_from_module(module: Module<'a>, span: Option<Span>) -> Self {
         NameBinding {
-            modifiers: DefModifiers::IMPORTABLE,
             kind: NameBindingKind::Module(module),
             span: span,
             vis: module.vis,
@@ -973,10 +960,6 @@ impl<'a> NameBinding<'a> {
         }
     }
 
-    fn defined_with(&self, modifiers: DefModifiers) -> bool {
-        self.modifiers.contains(modifiers)
-    }
-
     fn is_pseudo_public(&self) -> bool {
         self.pseudo_vis() == ty::Visibility::Public
     }
@@ -1003,6 +986,20 @@ impl<'a> NameBinding<'a> {
             _ => false,
         }
     }
+
+    fn is_glob_import(&self) -> bool {
+        match self.kind {
+            NameBindingKind::Import { directive, .. } => directive.is_glob(),
+            _ => false,
+        }
+    }
+
+    fn is_importable(&self) -> bool {
+        match self.def().unwrap() {
+            Def::AssociatedConst(..) | Def::Method(..) | Def::AssociatedTy(..) => false,
+            _ => true,
+        }
+    }
 }
 
 /// Interns the names of the primitive types.
@@ -1228,12 +1225,13 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             self.used_crates.insert(krate);
         }
 
-        let (import_id, privacy_error) = match binding.kind {
+        let (directive, privacy_error) = match binding.kind {
-            NameBindingKind::Import { id, ref privacy_error, .. } => (id, privacy_error),
+            NameBindingKind::Import { directive, ref privacy_error, .. } =>
+                (directive, privacy_error),
             _ => return,
         };
 
-        self.used_imports.insert((import_id, ns));
+        self.used_imports.insert((directive.id, ns));
         if let Some(error) = privacy_error.as_ref() {
             self.privacy_errors.push((**error).clone());
         }
@@ -1241,14 +1239,14 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         if !self.make_glob_map {
             return;
         }
-        if self.glob_map.contains_key(&import_id) {
+        if self.glob_map.contains_key(&directive.id) {
-            self.glob_map.get_mut(&import_id).unwrap().insert(name);
+            self.glob_map.get_mut(&directive.id).unwrap().insert(name);
             return;
         }
 
         let mut new_set = FnvHashSet();
         new_set.insert(name);
-        self.glob_map.insert(import_id, new_set);
+        self.glob_map.insert(directive.id, new_set);
     }
 
     fn get_trait_name(&self, did: DefId) -> Name {

src/librustc_resolve/resolve_imports.rs

@@ -10,7 +10,6 @@
 
 use self::ImportDirectiveSubclass::*;
 
-use DefModifiers;
 use Module;
 use Namespace::{self, TypeNS, ValueNS};
 use {NameBinding, NameBindingKind, PrivacyError};
@@ -59,11 +58,11 @@ impl ImportDirectiveSubclass {
 /// One import directive.
 #[derive(Debug,Clone)]
 pub struct ImportDirective<'a> {
+    pub id: NodeId,
     module_path: Vec<Name>,
     target_module: Cell<Option<Module<'a>>>, // the resolution of `module_path`
     subclass: ImportDirectiveSubclass,
     span: Span,
-    id: NodeId,
     vis: ty::Visibility, // see note in ImportResolutionPerNamespace about how to use this
     is_prelude: bool,
 }
@@ -71,24 +70,22 @@ pub struct ImportDirective<'a> {
 impl<'a> ImportDirective<'a> {
     // 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(&self, binding: &'a NameBinding<'a>, privacy_error: Option<Box<PrivacyError<'a>>>)
+    fn import(&'a self, binding: &'a NameBinding<'a>, privacy_error: Option<Box<PrivacyError<'a>>>)
               -> NameBinding<'a> {
-        let mut modifiers = DefModifiers::IMPORTABLE;
-        if let GlobImport = self.subclass {
-            modifiers = modifiers | DefModifiers::GLOB_IMPORTED;
-        }
-
         NameBinding {
             kind: NameBindingKind::Import {
                 binding: binding,
-                id: self.id,
+                directive: self,
                 privacy_error: privacy_error,
             },
             span: Some(self.span),
-            modifiers: modifiers,
             vis: self.vis,
         }
     }
+
+    pub fn is_glob(&self) -> bool {
+        match self.subclass { ImportDirectiveSubclass::GlobImport => true, _ => false }
+    }
 }
 
 #[derive(Clone, Default)]
@@ -141,9 +138,9 @@ impl<'a> SingleImports<'a> {
 impl<'a> NameResolution<'a> {
     fn try_define(&mut self, binding: &'a NameBinding<'a>) -> Result<(), &'a NameBinding<'a>> {
         if let Some(old_binding) = self.binding {
-            if binding.defined_with(DefModifiers::GLOB_IMPORTED) {
+            if binding.is_glob_import() {
                 self.duplicate_globs.push(binding);
-            } else if old_binding.defined_with(DefModifiers::GLOB_IMPORTED) {
+            } else if old_binding.is_glob_import() {
                 self.duplicate_globs.push(old_binding);
                 self.binding = Some(binding);
             } else {
@@ -160,7 +157,7 @@ impl<'a> NameResolution<'a> {
     fn binding(&self) -> Option<&'a NameBinding<'a>> {
         self.binding.and_then(|binding| match self.single_imports {
             SingleImports::None => Some(binding),
-            _ if !binding.defined_with(DefModifiers::GLOB_IMPORTED) => Some(binding),
+            _ if !binding.is_glob_import() => Some(binding),
             _ => None, // The binding could be shadowed by a single import, so it is not known.
         })
     }
@@ -170,7 +167,7 @@ impl<'a> NameResolution<'a> {
     fn try_result(&self, ns: Namespace, allow_private_imports: bool)
                   -> Option<ResolveResult<&'a NameBinding<'a>>> {
         match self.binding {
-            Some(binding) if !binding.defined_with(DefModifiers::GLOB_IMPORTED) =>
+            Some(binding) if !binding.is_glob_import() =>
                 return Some(Success(binding)),
             _ => {} // Items and single imports are not shadowable
         };
@@ -337,7 +334,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_pseudo_public() { return }
+        if !binding.is_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));
         }
@@ -410,7 +407,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         // resolution for it so that later resolve stages won't complain.
         if let SingleImport { target, .. } = e.import_directive.subclass {
             let dummy_binding = self.resolver.arenas.alloc_name_binding(NameBinding {
-                modifiers: DefModifiers::GLOB_IMPORTED,
                 kind: NameBindingKind::Def(Def::Err),
                 span: None,
                 vis: ty::Visibility::Public,
@@ -517,7 +513,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
 
             determined.set(true);
             if let Success(binding) = *result {
-                if !binding.defined_with(DefModifiers::IMPORTABLE) {
+                if !binding.is_importable() {
                     let msg = format!("`{}` is not directly importable", target);
                     span_err!(self.resolver.session, directive.span, E0253, "{}", &msg);
                 }
@@ -662,7 +658,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_pseudo_public() {
+            if binding.is_importable() && binding.is_pseudo_public() {
                 let _ = module_.try_define_child(name, ns, directive.import(binding, None));
             }
         }
@@ -705,14 +701,14 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                 }
             }
 
-            if let NameBindingKind::Import { binding: orig_binding, id, .. } = binding.kind {
+            if let NameBindingKind::Import { binding: orig_binding, directive, .. } = binding.kind {
                 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);
                     let lint = lint::builtin::PRIVATE_IN_PUBLIC;
-                    self.resolver.session.add_lint(lint, id, binding.span.unwrap(), msg);
+                    self.resolver.session.add_lint(lint, directive.id, binding.span.unwrap(), msg);
                 }
             }
         }