Allow items to shadow glob imports

Author: jseyfried

src/librustc_resolve/build_reduced_graph.rs

@@ -359,6 +359,11 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
             // These items live in both the type and value namespaces.
             ItemStruct(ref struct_def, _) => {
+                let modifiers = match !struct_def.is_struct() {
+                    true => modifiers | DefModifiers::LINKED_NAMESPACES,
+                    false => modifiers,
+                };
+
                 // 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));
@@ -434,7 +439,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, '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::PUBLIC | DefModifiers::IMPORTABLE | variant_modifiers |
+                        DefModifiers::LINKED_NAMESPACES;
         let def = Def::Variant(item_id, self.ast_map.local_def_id(variant.node.data.id()));
 
         self.define(parent, name, ValueNS, (def, variant.span, modifiers));
@@ -523,7 +529,8 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                        final_ident);
                 // 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;
+                let modifiers = DefModifiers::PUBLIC | DefModifiers::IMPORTABLE |
+                                DefModifiers::LINKED_NAMESPACES;
                 self.try_define(new_parent, name, TypeNS, (def, DUMMY_SP, modifiers));
                 self.try_define(new_parent, name, ValueNS, (def, DUMMY_SP, modifiers));
                 if self.session.cstore.variant_kind(variant_id) == Some(VariantKind::Struct) {
@@ -577,8 +584,14 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
                 debug!("(building reduced graph for external crate) building type and value for \
                         {}",
                        final_ident);
+                let ctor_def_id = self.session.cstore.struct_ctor_def_id(def_id);
+                let modifiers = match ctor_def_id {
+                    Some(_) => modifiers | DefModifiers::LINKED_NAMESPACES,
+                    None => modifiers,
+                };
+
                 self.try_define(new_parent, name, TypeNS, (def, DUMMY_SP, modifiers));
-                if let Some(ctor_def_id) = self.session.cstore.struct_ctor_def_id(def_id) {
+                if let Some(ctor_def_id) = ctor_def_id {
                     let def = Def::Struct(ctor_def_id);
                     self.try_define(new_parent, name, ValueNS, (def, DUMMY_SP, modifiers));
                 }

src/librustc_resolve/lib.rs

@@ -873,6 +873,17 @@ impl<'a> ModuleS<'a> {
     // Define the name or return the existing binding if there is a collision.
     fn try_define_child(&self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>)
                         -> Result<(), &'a NameBinding<'a>> {
+        // If binding is glob imported and is an item that defines both namespaces,
+        // it will be shadowed by an existing non-imported item in either namespace.
+        if binding.defined_with(DefModifiers::GLOB_IMPORTED | DefModifiers::LINKED_NAMESPACES) {
+            for &ns in &[ValueNS, TypeNS] {
+                match self.children.borrow().get(&(name, ns)).cloned().unwrap_or_default().binding {
+                    Some(binding) if !binding.is_import() => return Ok(()),
+                    _ => {}
+                }
+            }
+        }
+
         let mut children = self.children.borrow_mut();
         let resolution = children.entry((name, ns)).or_insert_with(Default::default);
 
@@ -989,6 +1000,9 @@ bitflags! {
         const PRIVATE_VARIANT = 1 << 2,
         const PRELUDE = 1 << 3,
         const GLOB_IMPORTED = 1 << 4,
+        // A binding for a name in a namespace has this flag if the name is defined in both
+        // namespaces by the same non-import item (i.e. by a tuple struct, unit struct, or variant).
+        const LINKED_NAMESPACES = 1 << 5,
     }
 }
 

src/librustc_resolve/resolve_imports.rs

@@ -90,6 +90,9 @@ impl ImportDirective {
         if self.shadowable == Shadowable::Always {
             modifiers = modifiers | DefModifiers::PRELUDE;
         }
+        if binding.defined_with(DefModifiers::LINKED_NAMESPACES) {
+            modifiers = modifiers | DefModifiers::LINKED_NAMESPACES;
+        }
 
         NameBinding {
             kind: NameBindingKind::Import { binding: binding, id: self.id },
@@ -136,11 +139,9 @@ impl<'a> NameResolution<'a> {
             _ => { self.binding = Some(binding); return Ok(()); }
         };
 
-        // FIXME #31337: We currently allow items to shadow glob-imported re-exports.
+        // We currently allow items to shadow glob-imports.
         if !old_binding.is_import() && binding.defined_with(DefModifiers::GLOB_IMPORTED) {
-            if let NameBindingKind::Import { binding, .. } = binding.kind {
-                if binding.is_import() { return Ok(()); }
-            }
+            return Ok(());
         }
 
         Err(old_binding)

src/test/compile-fail/shadowed-glob-import.rs

@@ -0,0 +1,38 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+mod foo {
+    pub struct Bar;
+
+    pub enum E { V }
+    pub use self::E::V;
+
+    pub fn baz() -> bool {}
+    pub mod baz { pub fn f() {} }
+}
+
+use foo::*;
+fn Bar() {}
+struct V { x: i32 }
+fn baz() {}
+
+fn main() {
+    // The function `Bar` shadows the imported struct `Bar` in both namespaces
+    Bar();
+    let x: Bar = unimplemented!(); //~ ERROR use of undeclared type name `Bar`
+
+    // The struct `V` shadows the imported variant `V` in both namespaces
+    let _ = V { x: 0 };
+    let _ = V; //~ ERROR `V` is the name of a struct
+
+    // The function `baz` only shadows the imported name `baz` in the value namespace
+    let _: () = baz();
+    let _ = baz::f();
+}

src/test/compile-fail/variant-namespacing.rs

@@ -11,14 +11,14 @@
 // aux-build:variant-namespacing.rs
 
 extern crate variant_namespacing;
-pub use variant_namespacing::XE::*;
+pub use variant_namespacing::XE::{XStruct, XTuple, XUnit};
 //~^ ERROR import `XStruct` conflicts with type in this module
 //~| ERROR import `XStruct` conflicts with value in this module
 //~| ERROR import `XTuple` conflicts with type in this module
 //~| ERROR import `XTuple` conflicts with value in this module
 //~| ERROR import `XUnit` conflicts with type in this module
 //~| ERROR import `XUnit` conflicts with value in this module
-pub use E::*;
+pub use E::{Struct, Tuple, Unit};
 //~^ ERROR import `Struct` conflicts with type in this module
 //~| ERROR import `Struct` conflicts with value in this module
 //~| ERROR import `Tuple` conflicts with type in this module