]> git.lizzy.rs Git - rust.git/commitdiff
Fix issue #21546 and refactor NsDef
authorJeffrey Seyfried <jeffrey.seyfried@gmail.com>
Tue, 17 Nov 2015 09:10:41 +0000 (09:10 +0000)
committerJeffrey Seyfried <jeffrey.seyfried@gmail.com>
Tue, 17 Nov 2015 09:10:41 +0000 (09:10 +0000)
src/librustc_resolve/build_reduced_graph.rs
src/librustc_resolve/lib.rs
src/librustc_resolve/resolve_imports.rs
src/test/compile-fail/issue-21546.rs

index b70349bfe94a10ab21d417fd345c1191da0488f9..b519a729918635cb19d34f45946328202dafb08d 100644 (file)
@@ -27,7 +27,6 @@
 use {resolve_error, ResolutionError};
 
 use self::DuplicateCheckingMode::*;
-use self::NamespaceError::*;
 
 use rustc::metadata::csearch;
 use rustc::metadata::decoder::{DefLike, DlDef, DlField, DlImpl};
 // another item exists with the same name in some namespace.
 #[derive(Copy, Clone, PartialEq)]
 enum DuplicateCheckingMode {
-    ForbidDuplicateModules,
-    ForbidDuplicateTypesAndModules,
+    ForbidDuplicateTypes,
     ForbidDuplicateValues,
     ForbidDuplicateTypesAndValues,
     OverwriteDuplicates,
 }
 
-#[derive(Copy, Clone, PartialEq)]
-enum NamespaceError {
-    NoError,
-    ModuleError,
-    TypeError,
-    ValueError,
-}
-
-fn namespace_error_to_string(ns: NamespaceError) -> &'static str {
-    match ns {
-        NoError => "",
-        ModuleError | TypeError => "type or module",
-        ValueError => "value",
-    }
-}
-
 struct GraphBuilder<'a, 'b: 'a, 'tcx: 'b> {
     resolver: &'a mut Resolver<'b, 'tcx>,
 }
@@ -112,16 +94,9 @@ fn build_reduced_graph(self, krate: &hir::Crate) {
         visit::walk_crate(&mut visitor, krate);
     }
 
-    /// Adds a new child item to the module definition of the parent node and
-    /// returns its corresponding name bindings as well as the current parent.
-    /// Or, if we're inside a block, creates (or reuses) an anonymous module
-    /// corresponding to the innermost block ID and returns the name bindings
-    /// as well as the newly-created parent.
-    ///
-    /// # Panics
-    ///
-    /// Panics if this node does not have a module definition and we are not inside
-    /// a block.
+    /// Adds a new child item to the module definition of the parent node,
+    /// or if there is already a child, does duplicate checking on the child.
+    /// Returns the child's corresponding name bindings.
     fn add_child(&self,
                  name: Name,
                  parent: &Rc<Module>,
@@ -129,10 +104,6 @@ fn add_child(&self,
                  // For printing errors
                  sp: Span)
                  -> NameBindings {
-        // If this is the immediate descendant of a module, then we add the
-        // child name directly. Otherwise, we create or reuse an anonymous
-        // module and add the child to that.
-
         self.check_for_conflicts_between_external_crates_and_items(&**parent, name, sp);
 
         // Add or reuse the child.
@@ -146,79 +117,33 @@ fn add_child(&self,
             Some(child) => {
                 // Enforce the duplicate checking mode:
                 //
-                // * If we're requesting duplicate module checking, check that
-                //   there isn't a module in the module with the same name.
-                //
                 // * If we're requesting duplicate type checking, check that
-                //   there isn't a type in the module with the same name.
+                //   the name isn't defined in the type namespace.
                 //
                 // * If we're requesting duplicate value checking, check that
-                //   there isn't a value in the module with the same name.
+                //   the name isn't defined in the value namespace.
                 //
-                // * If we're requesting duplicate type checking and duplicate
-                //   value checking, check that there isn't a duplicate type
-                //   and a duplicate value with the same name.
+                // * If we're requesting duplicate type and value checking,
+                //   check that the name isn't defined in either namespace.
                 //
                 // * If no duplicate checking was requested at all, do
                 //   nothing.
 
-                let mut duplicate_type = NoError;
                 let ns = match duplicate_checking_mode {
-                    ForbidDuplicateModules => {
-                        if child.get_module_if_available().is_some() {
-                            duplicate_type = ModuleError;
-                        }
-                        Some(TypeNS)
-                    }
-                    ForbidDuplicateTypesAndModules => {
-                        if child.type_ns.defined() {
-                            duplicate_type = TypeError;
-                        }
-                        Some(TypeNS)
-                    }
-                    ForbidDuplicateValues => {
-                        if child.value_ns.defined() {
-                            duplicate_type = ValueError;
-                        }
-                        Some(ValueNS)
-                    }
-                    ForbidDuplicateTypesAndValues => {
-                        let mut n = None;
-                        match child.type_ns.def() {
-                            Some(DefMod(_)) | None => {}
-                            Some(_) => {
-                                n = Some(TypeNS);
-                                duplicate_type = TypeError;
-                            }
-                        }
-                        if child.value_ns.defined() {
-                            duplicate_type = ValueError;
-                            n = Some(ValueNS);
-                        }
-                        n
-                    }
-                    OverwriteDuplicates => None,
+                    ForbidDuplicateTypes if child.type_ns.defined() => TypeNS,
+                    ForbidDuplicateValues if child.value_ns.defined() => ValueNS,
+                    ForbidDuplicateTypesAndValues if child.type_ns.defined() => TypeNS,
+                    ForbidDuplicateTypesAndValues if child.value_ns.defined() => ValueNS,
+                    _ => return child,
                 };
-                if duplicate_type != NoError {
-                    // Return an error here by looking up the namespace that
-                    // had the duplicate.
-                    let ns = ns.unwrap();
-                    resolve_error(
-                        self,
-                        sp,
-                        ResolutionError::DuplicateDefinition(
-                            namespace_error_to_string(duplicate_type),
-                            name)
-                    );
-                    {
-                        let r = child[ns].span();
-                        if let Some(sp) = r {
-                            self.session.span_note(sp,
-                                                   &format!("first definition of {} `{}` here",
-                                      namespace_error_to_string(duplicate_type),
-                                      name));
-                        }
-                    }
+
+                // Record an error here by looking up the namespace that had the duplicate
+                let ns_str = match ns { TypeNS => "type or module", ValueNS => "value" };
+                resolve_error(self, sp, ResolutionError::DuplicateDefinition(ns_str, name));
+
+                if let Some(sp) = child[ns].span() {
+                    let note = format!("first definition of {} `{}` here", ns_str, name);
+                    self.session.span_note(sp, &note);
                 }
                 child
             }
@@ -409,29 +334,7 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, parent: &Rc<Module>) ->
             }
 
             ItemMod(..) => {
-                let child = parent.children.borrow().get(&name).cloned();
-                if let Some(child) = child {
-                    // check if there's struct of the same name already defined
-                    if child.type_ns.defined() &&
-                       child.get_module_if_available().is_none() {
-                        self.session.span_warn(sp,
-                                               &format!("duplicate definition of {} `{}`. \
-                                                         Defining a module and a struct with \
-                                                         the same name will be disallowed soon.",
-                                                        namespace_error_to_string(TypeError),
-                                                        name));
-                        {
-                            let r = child.type_ns.span();
-                            if let Some(sp) = r {
-                                self.session.span_note(sp,
-                                                       &format!("first definition of {} `{}` here",
-                                          namespace_error_to_string(TypeError),
-                                          name));
-                            }
-                        }
-                    }
-                }
-                let name_bindings = self.add_child(name, parent, ForbidDuplicateModules, sp);
+                let name_bindings = self.add_child(name, parent, ForbidDuplicateTypes, sp);
 
                 let parent_link = self.get_parent_link(parent, name);
                 let def = DefMod(self.ast_map.local_def_id(item.id));
@@ -469,7 +372,7 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, parent: &Rc<Module>) ->
             ItemTy(..) => {
                 let name_bindings = self.add_child(name,
                                                    parent,
-                                                   ForbidDuplicateTypesAndModules,
+                                                   ForbidDuplicateTypes,
                                                    sp);
 
                 let parent_link = self.get_parent_link(parent, name);
@@ -481,7 +384,7 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, parent: &Rc<Module>) ->
             ItemEnum(ref enum_definition, _) => {
                 let name_bindings = self.add_child(name,
                                                    parent,
-                                                   ForbidDuplicateTypesAndModules,
+                                                   ForbidDuplicateTypes,
                                                    sp);
 
                 let parent_link = self.get_parent_link(parent, name);
@@ -501,31 +404,8 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, parent: &Rc<Module>) ->
             ItemStruct(ref struct_def, _) => {
                 // Adding to both Type and Value namespaces or just Type?
                 let (forbid, ctor_id) = if struct_def.is_struct() {
-                    (ForbidDuplicateTypesAndModules, None)
+                    (ForbidDuplicateTypes, None)
                 } else {
-                    let child = parent.children.borrow().get(&name).cloned();
-                    if let Some(child) = child {
-                        // check if theres a DefMod
-                        if let Some(DefMod(_)) = child.type_ns.def() {
-                            self.session.span_warn(sp,
-                                                   &format!("duplicate definition of {} `{}`. \
-                                                             Defining a module and a struct \
-                                                             with the same name will be \
-                                                             disallowed soon.",
-                                                            namespace_error_to_string(TypeError),
-                                                            name));
-                            {
-                                let r = child.type_ns.span();
-                                if let Some(sp) = r {
-                                    self.session
-                                        .span_note(sp,
-                                                   &format!("first definition of {} `{}` here",
-                                                            namespace_error_to_string(TypeError),
-                                                            name));
-                                }
-                            }
-                        }
-                    }
                     (ForbidDuplicateTypesAndValues, Some(struct_def.id()))
                 };
 
@@ -566,7 +446,7 @@ fn build_reduced_graph_for_item(&mut self, item: &Item, parent: &Rc<Module>) ->
             ItemTrait(_, _, _, ref items) => {
                 let name_bindings = self.add_child(name,
                                                    parent,
-                                                   ForbidDuplicateTypesAndModules,
+                                                   ForbidDuplicateTypes,
                                                    sp);
 
                 let def_id = self.ast_map.local_def_id(item.id);
index 18e2b66d3fb2e8e863ff06c8264fde2d7b92db04..6a49db8a14665ac7aa1d2cdb3e340201681aa485 100644 (file)
@@ -904,17 +904,20 @@ fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
     }
 }
 
-// Records a possibly-private definition.
-// FIXME once #21546 is resolved, the def and module fields will never both be Some,
-// so they can be refactored into something like Result<Def, Rc<Module>>.
+// Records a possibly-private value, type, or module definition.
 #[derive(Debug)]
 struct NsDef {
     modifiers: DefModifiers, // see note in ImportResolution about how to use this
-    def: Option<Def>,
-    module: Option<Rc<Module>>,
+    def_or_module: DefOrModule,
     span: Option<Span>,
 }
 
+#[derive(Debug)]
+enum DefOrModule {
+    Def(Def),
+    Module(Rc<Module>),
+}
+
 impl NsDef {
     fn create_from_module(module: Rc<Module>, span: Option<Span>) -> Self {
         let modifiers = if module.is_public {
@@ -923,14 +926,24 @@ fn create_from_module(module: Rc<Module>, span: Option<Span>) -> Self {
             DefModifiers::empty()
         } | DefModifiers::IMPORTABLE;
 
-        NsDef { modifiers: modifiers, def: None, module: Some(module), span: span }
+        NsDef { modifiers: modifiers, def_or_module: DefOrModule::Module(module), span: span }
+    }
+
+    fn create_from_def(def: Def, modifiers: DefModifiers, span: Option<Span>) -> Self {
+        NsDef { modifiers: modifiers, def_or_module: DefOrModule::Def(def), span: span }
+    }
+
+    fn module(&self) -> Option<Rc<Module>> {
+        match self.def_or_module {
+            DefOrModule::Module(ref module) => Some(module.clone()),
+            DefOrModule::Def(_) => None,
+        }
     }
 
     fn def(&self) -> Option<Def> {
-        match (self.def, &self.module) {
-            (def @ Some(_), _) => def,
-            (_, &Some(ref module)) => module.def.get(),
-            _ => panic!("NsDef has neither a Def nor a Module"),
+        match self.def_or_module {
+            DefOrModule::Def(def) => Some(def),
+            DefOrModule::Module(ref module) => module.def.get(),
         }
     }
 }
@@ -964,10 +977,10 @@ fn and_then<T, F: Fn(&NsDef) -> Option<T>>(&self, f: F) -> Option<T> {
 
     fn borrow(&self) -> ::std::cell::Ref<Option<NsDef>> { self.0.borrow() }
 
-    // Lifted versions of the NsDef fields and method
+    // Lifted versions of the NsDef methods and fields
     fn def(&self) -> Option<Def>           { self.and_then(NsDef::def) }
+    fn module(&self) -> Option<Rc<Module>> { self.and_then(NsDef::module) }
     fn span(&self) -> Option<Span>         { self.and_then(|def| def.span) }
-    fn module(&self) -> Option<Rc<Module>> { self.and_then(|def| def.module.clone()) }
     fn modifiers(&self) -> Option<DefModifiers> { self.and_then(|def| Some(def.modifiers)) }
 
     fn defined(&self) -> bool { self.borrow().is_some() }
@@ -1029,23 +1042,14 @@ fn define_module(&self,
 
     /// Records a type definition.
     fn define_type(&self, def: Def, sp: Span, modifiers: DefModifiers) {
-        debug!("defining type for def {:?} with modifiers {:?}",
-               def,
-               modifiers);
-        // Merges the type with the existing type def or creates a new one.
-        self.type_ns.set(NsDef {
-            modifiers: modifiers, def: Some(def), module: self.type_ns.module(), span: Some(sp)
-        });
+        debug!("defining type for def {:?} with modifiers {:?}", def, modifiers);
+        self.type_ns.set(NsDef::create_from_def(def, modifiers, Some(sp)));
     }
 
     /// Records a value definition.
     fn define_value(&self, def: Def, sp: Span, modifiers: DefModifiers) {
-        debug!("defining value for def {:?} with modifiers {:?}",
-               def,
-               modifiers);
-        self.value_ns.set(NsDef {
-            modifiers: modifiers, def: Some(def), module: None, span: Some(sp)
-        });
+        debug!("defining value for def {:?} with modifiers {:?}", def, modifiers);
+        self.value_ns.set(NsDef::create_from_def(def, modifiers, Some(sp)));
     }
 
     /// Returns the module node if applicable.
@@ -1524,7 +1528,7 @@ fn resolve_item_in_lexical_scope(&mut self,
                     self.used_imports.insert((id, namespace));
                     self.record_import_use(id, name);
                     if let Some(DefId{krate: kid, ..}) = target.target_module.def_id() {
-                         self.used_crates.insert(kid);
+                        self.used_crates.insert(kid);
                     }
                     return Success((target, false));
                 }
index 14f8287a5f875615e2f7b99826d7eea1a9cf7d4c..24a21bcdfb1b09d176a29a70f37c750fc94b43e4 100644 (file)
@@ -1039,7 +1039,7 @@ fn check_for_conflicts_between_imports_and_items(&mut self,
         match import_resolution.type_target {
             Some(ref target) if target.shadowable != Shadowable::Always => {
                 if let Some(ref ty) = *name_bindings.type_ns.borrow() {
-                    let (what, note) = match ty.module.clone() {
+                    let (what, note) = match ty.module() {
                         Some(ref module) if module.is_normal() =>
                             ("existing submodule", "note conflicting module here"),
                         Some(ref module) if module.is_trait() =>
index bb1bcd4e8717d6c2e921fa3df89a6df7c7b2ab64..535630e0824ca571725c9c766d59e4983a817b7e 100644 (file)
@@ -16,7 +16,7 @@ mod Foo { }
 
 #[allow(dead_code)]
 struct Foo;
-//~^ WARNING duplicate definition of type or module `Foo`
+//~^ ERROR duplicate definition of type or module `Foo`
 
 
 #[allow(non_snake_case)]
@@ -25,7 +25,7 @@ mod Bar { }
 
 #[allow(dead_code)]
 struct Bar(i32);
-//~^ WARNING duplicate definition of type or module `Bar`
+//~^ ERROR duplicate definition of type or module `Bar`
 
 
 #[allow(dead_code)]
@@ -34,7 +34,7 @@ mod Bar { }
 
 #[allow(non_snake_case)]
 mod Baz { }
-//~^ WARNING duplicate definition of type or module `Baz`
+//~^ ERROR duplicate definition of type or module `Baz`
 
 
 #[allow(dead_code)]
@@ -43,7 +43,7 @@ struct Qux { x: bool }
 
 #[allow(non_snake_case)]
 mod Qux { }
-//~^ WARNING duplicate definition of type or module `Qux`
+//~^ ERROR duplicate definition of type or module `Qux`
 
 
 #[allow(dead_code)]
@@ -52,7 +52,7 @@ mod Qux { }
 
 #[allow(non_snake_case)]
 mod Quux { }
-//~^ WARNING duplicate definition of type or module `Quux`
+//~^ ERROR duplicate definition of type or module `Quux`
 
 
 #[allow(dead_code)]