Auto merge of #30294 - jseyfried:fix_shadowed_use_visibility, r=nrc

This fixes a bug in which the visibility of a use declaration defining a name in one namespace (e.g. the value namespace) is overridden by a later use declaration defining the same name in the other namespace (e.g. the type namespace). For example,
```rust
fn f() {}
pub mod bar {}

mod foo {
    use f; // This import should not be visible outside `foo`,
    pub use bar as f; // but it visible outside of `foo` because of this import.
}

fn main() { foo::f(); }
```
As the example demonstrates, this is a [breaking-change], but it looks unlikely to cause breakage in practice, and any breakage can be fixed by correcting visibility modifiers.

6 files changed, 168 insertions, 163 deletions

diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index 766a6d361dd..0deef91a0f6 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -16,7 +16,7 @@
 use DefModifiers;
 use resolve_imports::ImportDirective;
 use resolve_imports::ImportDirectiveSubclass::{self, SingleImport, GlobImport};
-use resolve_imports::ImportResolution;
+use resolve_imports::{ImportResolution, ImportResolutionPerNamespace};
 use Module;
 use Namespace::{TypeNS, ValueNS};
 use NameBindings;
@@ -822,22 +822,23 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {
 
                 let mut import_resolutions = module_.import_resolutions.borrow_mut();
                 match import_resolutions.get_mut(&target) {
-                    Some(resolution) => {
+                    Some(resolution_per_ns) => {
                         debug!("(building import directive) bumping reference");
-                        resolution.outstanding_references += 1;
+                        resolution_per_ns.outstanding_references += 1;
 
                         // the source of this name is different now
-                        resolution.type_id = id;
-                        resolution.value_id = id;
-                        resolution.is_public = is_public;
+                        let resolution =
+                            ImportResolution { id: id, is_public: is_public, target: None };
+                        resolution_per_ns[TypeNS] = resolution.clone();
+                        resolution_per_ns[ValueNS] = resolution;
                         return;
                     }
                     None => {}
                 }
                 debug!("(building import directive) creating new");
-                let mut resolution = ImportResolution::new(id, is_public);
-                resolution.outstanding_references = 1;
-                import_resolutions.insert(target, resolution);
+                let mut import_resolution_per_ns = ImportResolutionPerNamespace::new(id, is_public);
+                import_resolution_per_ns.outstanding_references = 1;
+                import_resolutions.insert(target, import_resolution_per_ns);
             }
             GlobImport => {
                 // Set the glob flag. This tells us that we don't know the
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index ddada1b513d..c32acb7bb26 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -96,7 +96,7 @@ use std::mem::replace;
 use std::rc::{Rc, Weak};
 use std::usize;
 
-use resolve_imports::{Target, ImportDirective, ImportResolution};
+use resolve_imports::{Target, ImportDirective, ImportResolutionPerNamespace};
 use resolve_imports::Shadowable;
 
 // NB: This module needs to be declared first so diagnostics are
@@ -328,7 +328,7 @@ fn resolve_error<'b, 'a: 'b, 'tcx: 'a>(resolver: &'b Resolver<'a, 'tcx>,
                                              .import_resolutions
                                              .borrow()
                                              .get(&name) {
-                let item = resolver.ast_map.expect_item(directive.value_id);
+                let item = resolver.ast_map.expect_item(directive.value_ns.id);
                 resolver.session.span_note(item.span, "constant imported here");
             }
         }
@@ -793,7 +793,7 @@ pub struct Module {
     anonymous_children: RefCell<NodeMap<Rc<Module>>>,
 
     // The status of resolving each import in this module.
-    import_resolutions: RefCell<HashMap<Name, ImportResolution>>,
+    import_resolutions: RefCell<HashMap<Name, ImportResolutionPerNamespace>>,
 
     // The number of unresolved globs that this module exports.
     glob_count: Cell<usize>,
@@ -912,7 +912,7 @@ bitflags! {
 // Records a possibly-private value, type, or module definition.
 #[derive(Debug)]
 struct NsDef {
-    modifiers: DefModifiers, // see note in ImportResolution about how to use this
+    modifiers: DefModifiers, // see note in ImportResolutionPerNamespace about how to use this
     def_or_module: DefOrModule,
     span: Option<Span>,
 }
@@ -1491,7 +1491,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         // adjacent import statements are processed as though they mutated the
         // current scope.
         if let Some(import_resolution) = module_.import_resolutions.borrow().get(&name) {
-            match (*import_resolution).target_for_namespace(namespace) {
+            match import_resolution[namespace].target.clone() {
                 None => {
                     // Not found; continue.
                     debug!("(resolving item in lexical scope) found import resolution, but not \
@@ -1501,7 +1501,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                 Some(target) => {
                     debug!("(resolving item in lexical scope) using import resolution");
                     // track used imports and extern crates as well
-                    let id = import_resolution.id(namespace);
+                    let id = import_resolution[namespace].id;
                     self.used_imports.insert((id, namespace));
                     self.record_import_use(id, name);
                     if let Some(DefId{krate: kid, ..}) = target.target_module.def_id() {
@@ -1712,13 +1712,15 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
         // Check the list of resolved imports.
         match module_.import_resolutions.borrow().get(&name) {
-            Some(import_resolution) if allow_private_imports || import_resolution.is_public => {
+            Some(import_resolution) if allow_private_imports ||
+                                       import_resolution[namespace].is_public => {
 
-                if import_resolution.is_public && import_resolution.outstanding_references != 0 {
+                if import_resolution[namespace].is_public &&
+                   import_resolution.outstanding_references != 0 {
                     debug!("(resolving name in module) import unresolved; bailing out");
                     return Indeterminate;
                 }
-                match import_resolution.target_for_namespace(namespace) {
+                match import_resolution[namespace].target.clone() {
                     None => {
                         debug!("(resolving name in module) name found, but not in namespace {:?}",
                                namespace);
@@ -1726,7 +1728,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                     Some(target) => {
                         debug!("(resolving name in module) resolved to import");
                         // track used imports and extern crates as well
-                        let id = import_resolution.id(namespace);
+                        let id = import_resolution[namespace].id;
                         self.used_imports.insert((id, namespace));
                         self.record_import_use(id, name);
                         if let Some(DefId{krate: kid, ..}) = target.target_module.def_id() {
@@ -3651,9 +3653,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
 
             // Look for imports.
             for (_, import) in search_module.import_resolutions.borrow().iter() {
-                let target = match import.target_for_namespace(TypeNS) {
+                let target = match import.type_ns.target {
                     None => continue,
-                    Some(target) => target,
+                    Some(ref target) => target,
                 };
                 let did = match target.binding.def() {
                     Some(DefTrait(trait_def_id)) => trait_def_id,
@@ -3661,7 +3663,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
                 };
                 if self.trait_item_map.contains_key(&(name, did)) {
                     add_trait_info(&mut found_traits, did, name);
-                    let id = import.type_id;
+                    let id = import.type_ns.id;
                     self.used_imports.insert((id, TypeNS));
                     let trait_name = self.get_trait_name(did);
                     self.record_import_use(id, trait_name);
@@ -3734,7 +3736,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
         let import_resolutions = module_.import_resolutions.borrow();
         for (&name, import_resolution) in import_resolutions.iter() {
             let value_repr;
-            match import_resolution.target_for_namespace(ValueNS) {
+            match import_resolution.value_ns.target {
                 None => {
                     value_repr = "".to_string();
                 }
@@ -3745,7 +3747,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
             }
 
             let type_repr;
-            match import_resolution.target_for_namespace(TypeNS) {
+            match import_resolution.type_ns.target {
                 None => {
                     type_repr = "".to_string();
                 }
diff --git a/src/librustc_resolve/record_exports.rs b/src/librustc_resolve/record_exports.rs
index 3a6a5a031b6..59cf83e91d2 100644
--- a/src/librustc_resolve/record_exports.rs
+++ b/src/librustc_resolve/record_exports.rs
@@ -130,13 +130,14 @@ impl<'a, 'b, 'tcx> ExportRecorder<'a, 'b, 'tcx> {
 
     fn add_exports_for_module(&mut self, exports: &mut Vec<Export>, module_: &Module) {
         for (name, import_resolution) in module_.import_resolutions.borrow().iter() {
-            if !import_resolution.is_public {
-                continue;
-            }
             let xs = [TypeNS, ValueNS];
             for &ns in &xs {
-                match import_resolution.target_for_namespace(ns) {
-                    Some(target) => {
+                if !import_resolution[ns].is_public {
+                    continue;
+                }
+
+                match import_resolution[ns].target {
+                    Some(ref target) => {
                         debug!("(computing exports) maybe export '{}'", name);
                         self.add_export_of_namebinding(exports, *name, &target.binding)
                     }
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
index 460c851e13f..fd471893acd 100644
--- a/src/librustc_resolve/resolve_imports.rs
+++ b/src/librustc_resolve/resolve_imports.rs
@@ -58,7 +58,7 @@ pub struct ImportDirective {
     pub subclass: ImportDirectiveSubclass,
     pub span: Span,
     pub id: NodeId,
-    pub is_public: bool, // see note in ImportResolution about how to use this
+    pub is_public: bool, // see note in ImportResolutionPerNamespace about how to use this
     pub shadowable: Shadowable,
 }
 
@@ -102,80 +102,61 @@ impl Target {
     }
 }
 
-/// An ImportResolution represents a particular `use` directive.
 #[derive(Debug)]
+/// An ImportResolutionPerNamespace records what we know about an imported name.
+/// More specifically, it records the number of unresolved `use` directives that import the name,
+/// and for each namespace, it records the `use` directive importing the name in the namespace
+/// and the `Target` to which the name in the namespace resolves (if applicable).
+/// Different `use` directives may import the same name in different namespaces.
+pub struct ImportResolutionPerNamespace {
+    // When outstanding_references reaches zero, outside modules can count on the targets being
+    // correct. Before then, all bets are off; future `use` directives could override the name.
+    // Since shadowing is forbidden, the only way outstanding_references > 1 in a legal program
+    // is if the name is imported by exactly two `use` directives, one of which resolves to a
+    // value and the other of which resolves to a type.
+    pub outstanding_references: usize,
+    pub type_ns: ImportResolution,
+    pub value_ns: ImportResolution,
+}
+
+/// Records what we know about an imported name in a namespace (see `ImportResolutionPerNamespace`).
+#[derive(Clone,Debug)]
 pub struct ImportResolution {
-    /// Whether this resolution came from a `use` or a `pub use`. Note that this
-    /// should *not* be used whenever resolution is being performed. Privacy
-    /// testing occurs during a later phase of compilation.
+    /// Whether the name in the namespace was imported with a `use` or a `pub use`.
     pub is_public: bool,
 
-    // The number of outstanding references to this name. When this reaches
-    // zero, outside modules can count on the targets being correct. Before
-    // then, all bets are off; future imports could override this name.
-    // Note that this is usually either 0 or 1 - shadowing is forbidden the only
-    // way outstanding_references is > 1 in a legal program is if the name is
-    // used in both namespaces.
-    pub outstanding_references: usize,
+    /// Resolution of the name in the namespace
+    pub target: Option<Target>,
 
-    /// The value that this `use` directive names, if there is one.
-    pub value_target: Option<Target>,
-    /// The source node of the `use` directive leading to the value target
-    /// being non-none
-    pub value_id: NodeId,
-
-    /// The type that this `use` directive names, if there is one.
-    pub type_target: Option<Target>,
-    /// The source node of the `use` directive leading to the type target
-    /// being non-none
-    pub type_id: NodeId,
+    /// The source node of the `use` directive
+    pub id: NodeId,
 }
 
-impl ImportResolution {
-    pub fn new(id: NodeId, is_public: bool) -> ImportResolution {
-        ImportResolution {
-            type_id: id,
-            value_id: id,
-            outstanding_references: 0,
-            value_target: None,
-            type_target: None,
-            is_public: is_public,
-        }
+impl ::std::ops::Index<Namespace> for ImportResolutionPerNamespace {
+    type Output = ImportResolution;
+    fn index(&self, ns: Namespace) -> &ImportResolution {
+        match ns { TypeNS => &self.type_ns, ValueNS => &self.value_ns }
     }
+}
 
-    pub fn target_for_namespace(&self, namespace: Namespace) -> Option<Target> {
-        match namespace {
-            TypeNS => self.type_target.clone(),
-            ValueNS => self.value_target.clone(),
-        }
+impl ::std::ops::IndexMut<Namespace> for ImportResolutionPerNamespace {
+    fn index_mut(&mut self, ns: Namespace) -> &mut ImportResolution {
+        match ns { TypeNS => &mut self.type_ns, ValueNS => &mut self.value_ns }
     }
+}
 
-    pub fn id(&self, namespace: Namespace) -> NodeId {
-        match namespace {
-            TypeNS => self.type_id,
-            ValueNS => self.value_id,
+impl ImportResolutionPerNamespace {
+    pub fn new(id: NodeId, is_public: bool) -> Self {
+        let resolution = ImportResolution { id: id, is_public: is_public, target: None };
+        ImportResolutionPerNamespace {
+            outstanding_references: 0, type_ns: resolution.clone(), value_ns: resolution,
         }
     }
 
     pub fn shadowable(&self, namespace: Namespace) -> Shadowable {
-        let target = self.target_for_namespace(namespace);
-        if target.is_none() {
-            return Shadowable::Always;
-        }
-
-        target.unwrap().shadowable
-    }
-
-    pub fn set_target_and_id(&mut self, namespace: Namespace, target: Option<Target>, id: NodeId) {
-        match namespace {
-            TypeNS => {
-                self.type_target = target;
-                self.type_id = id;
-            }
-            ValueNS => {
-                self.value_target = target;
-                self.value_id = id;
-            }
+        match self[namespace].target {
+            Some(ref target) => target.shadowable,
+            None => Shadowable::Always,
         }
     }
 }
@@ -523,18 +504,18 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                     Some(import_resolution) if import_resolution.outstanding_references == 0 => {
 
                         fn get_binding(this: &mut Resolver,
-                                       import_resolution: &ImportResolution,
+                                       import_resolution: &ImportResolutionPerNamespace,
                                        namespace: Namespace,
                                        source: Name)
                                        -> NamespaceResult {
 
                             // Import resolutions must be declared with "pub"
                             // in order to be exported.
-                            if !import_resolution.is_public {
+                            if !import_resolution[namespace].is_public {
                                 return UnboundResult;
                             }
 
-                            match import_resolution.target_for_namespace(namespace) {
+                            match import_resolution[namespace].target.clone() {
                                 None => {
                                     return UnboundResult;
                                 }
@@ -545,7 +526,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                                 }) => {
                                     debug!("(resolving single import) found import in ns {:?}",
                                            namespace);
-                                    let id = import_resolution.id(namespace);
+                                    let id = import_resolution[namespace].id;
                                     // track used imports and extern crates as well
                                     this.used_imports.insert((id, namespace));
                                     this.record_import_use(id, source);
@@ -567,14 +548,14 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                                                        import_resolution,
                                                        ValueNS,
                                                        source);
-                            value_used_reexport = import_resolution.is_public;
+                            value_used_reexport = import_resolution.value_ns.is_public;
                         }
                         if type_result.is_unknown() {
                             type_result = get_binding(self.resolver,
                                                       import_resolution,
                                                       TypeNS,
                                                       source);
-                            type_used_reexport = import_resolution.is_public;
+                            type_used_reexport = import_resolution.type_ns.is_public;
                         }
 
                     }
@@ -663,11 +644,13 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                                                              directive.span,
                                                              target);
 
-                        let target = Some(Target::new(target_module.clone(),
-                                                      name_binding.clone(),
-                                                      directive.shadowable));
-                        import_resolution.set_target_and_id(namespace, target, directive.id);
-                        import_resolution.is_public = directive.is_public;
+                        import_resolution[namespace] = ImportResolution {
+                            target: Some(Target::new(target_module.clone(),
+                                                     name_binding.clone(),
+                                                     directive.shadowable)),
+                            id: directive.id,
+                            is_public: directive.is_public
+                        };
                         *used_public = name_binding.is_public();
                     }
                     UnboundResult => {
@@ -702,7 +685,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         // Record what this import resolves to for later uses in documentation,
         // this may resolve to either a value or a type, but for documentation
         // purposes it's good enough to just favor one over the other.
-        let value_def_and_priv = import_resolution.value_target.as_ref().map(|target| {
+        let value_def_and_priv = import_resolution.value_ns.target.as_ref().map(|target| {
             let def = target.binding.def().unwrap();
             (def,
              if value_used_public {
@@ -711,7 +694,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                 DependsOn(def.def_id())
             })
         });
-        let type_def_and_priv = import_resolution.type_target.as_ref().map(|target| {
+        let type_def_and_priv = import_resolution.type_ns.target.as_ref().map(|target| {
             let def = target.binding.def().unwrap();
             (def,
              if type_used_public {
@@ -791,54 +774,26 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                    *name,
                    module_to_string(module_));
 
-            if !target_import_resolution.is_public {
-                debug!("(resolving glob import) nevermind, just kidding");
-                continue;
-            }
-
             // Here we merge two import resolutions.
             let mut import_resolutions = module_.import_resolutions.borrow_mut();
-            match import_resolutions.get_mut(name) {
-                Some(dest_import_resolution) => {
-                    // Merge the two import resolutions at a finer-grained
-                    // level.
-
-                    match target_import_resolution.value_target {
-                        None => {
-                            // Continue.
-                        }
-                        Some(ref value_target) => {
-                            self.check_for_conflicting_import(&dest_import_resolution,
-                                                              import_directive.span,
-                                                              *name,
-                                                              ValueNS);
-                            dest_import_resolution.value_target = Some(value_target.clone());
-                        }
-                    }
-                    match target_import_resolution.type_target {
-                        None => {
-                            // Continue.
-                        }
-                        Some(ref type_target) => {
-                            self.check_for_conflicting_import(&dest_import_resolution,
-                                                              import_directive.span,
-                                                              *name,
-                                                              TypeNS);
-                            dest_import_resolution.type_target = Some(type_target.clone());
-                        }
+            let mut dest_import_resolution = import_resolutions.entry(*name).or_insert_with(|| {
+                ImportResolutionPerNamespace::new(id, is_public)
+            });
+
+            for &ns in [TypeNS, ValueNS].iter() {
+                match target_import_resolution[ns].target {
+                    Some(ref target) if target_import_resolution[ns].is_public => {
+                        self.check_for_conflicting_import(&dest_import_resolution,
+                                                          import_directive.span,
+                                                          *name,
+                                                          ns);
+                        dest_import_resolution[ns] = ImportResolution {
+                            id: id, is_public: is_public, target: Some(target.clone())
+                        };
                     }
-                    dest_import_resolution.is_public = is_public;
-                    continue;
+                    _ => {}
                 }
-                None => {}
             }
-
-            // Simple: just copy the old import resolution.
-            let mut new_import_resolution = ImportResolution::new(id, is_public);
-            new_import_resolution.value_target = target_import_resolution.value_target.clone();
-            new_import_resolution.type_target = target_import_resolution.type_target.clone();
-
-            import_resolutions.insert(*name, new_import_resolution);
         }
 
         // Add all children from the containing module.
@@ -877,10 +832,9 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
         let is_public = import_directive.is_public;
 
         let mut import_resolutions = module_.import_resolutions.borrow_mut();
-        let dest_import_resolution = import_resolutions.entry(name)
-                                                       .or_insert_with(|| {
-                                                           ImportResolution::new(id, is_public)
-                                                       });
+        let dest_import_resolution = import_resolutions.entry(name).or_insert_with(|| {
+            ImportResolutionPerNamespace::new(id, is_public)
+        });
 
         debug!("(resolving glob import) writing resolution `{}` in `{}` to `{}`",
                name,
@@ -909,10 +863,13 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                                   "{}",
                                   msg);
                     } else {
-                        let target = Target::new(containing_module.clone(),
-                                                 name_bindings[namespace].clone(),
-                                                 import_directive.shadowable);
-                        dest_import_resolution.set_target_and_id(namespace, Some(target), id);
+                        dest_import_resolution[namespace] = ImportResolution {
+                            target: Some(Target::new(containing_module.clone(),
+                                                     name_bindings[namespace].clone(),
+                                                     import_directive.shadowable)),
+                            id: id,
+                            is_public: is_public
+                        };
                     }
                 }
             };
@@ -920,8 +877,6 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
             merge_child_item(TypeNS);
         }
 
-        dest_import_resolution.is_public = is_public;
-
         self.check_for_conflicts_between_imports_and_items(module_,
                                                            dest_import_resolution,
                                                            import_directive.span,
@@ -930,16 +885,16 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
 
     /// Checks that imported names and items don't have the same name.
     fn check_for_conflicting_import(&mut self,
-                                    import_resolution: &ImportResolution,
+                                    import_resolution: &ImportResolutionPerNamespace,
                                     import_span: Span,
                                     name: Name,
                                     namespace: Namespace) {
-        let target = import_resolution.target_for_namespace(namespace);
+        let target = &import_resolution[namespace].target;
         debug!("check_for_conflicting_import: {}; target exists: {}",
                name,
                target.is_some());
 
-        match target {
+        match *target {
             Some(ref target) if target.shadowable != Shadowable::Always => {
                 let ns_word = match namespace {
                     TypeNS => {
@@ -957,7 +912,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
                           "a {} named `{}` has already been imported in this module",
                           ns_word,
                           name);
-                let use_id = import_resolution.id(namespace);
+                let use_id = import_resolution[namespace].id;
                 let item = self.resolver.ast_map.expect_item(use_id);
                 // item is syntax::ast::Item;
                 span_note!(self.resolver.session,
@@ -983,14 +938,14 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
     /// Checks that imported names and items don't have the same name.
     fn check_for_conflicts_between_imports_and_items(&mut self,
                                                      module: &Module,
-                                                     import_resolution: &ImportResolution,
+                                                     import: &ImportResolutionPerNamespace,
                                                      import_span: Span,
                                                      name: Name) {
         // First, check for conflicts between imports and `extern crate`s.
         if module.external_module_children
                  .borrow()
                  .contains_key(&name) {
-            match import_resolution.type_target {
+            match import.type_ns.target {
                 Some(ref target) if target.shadowable != Shadowable::Always => {
                     let msg = format!("import `{0}` conflicts with imported crate in this module \
                                        (maybe you meant `use {0}::*`?)",
@@ -1011,7 +966,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
             Some(ref name_bindings) => (*name_bindings).clone(),
         };
 
-        match import_resolution.value_target {
+        match import.value_ns.target {
             Some(ref target) if target.shadowable != Shadowable::Always => {
                 if let Some(ref value) = *name_bindings.value_ns.borrow() {
                     span_err!(self.resolver.session,
@@ -1027,7 +982,7 @@ impl<'a, 'b:'a, 'tcx:'b> ImportResolver<'a, 'b, 'tcx> {
             Some(_) | None => {}
         }
 
-        match import_resolution.type_target {
+        match import.type_ns.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() {
diff --git a/src/test/compile-fail/shadowed-use-visibility.rs b/src/test/compile-fail/shadowed-use-visibility.rs
new file mode 100644
index 00000000000..1bf7f393384
--- /dev/null
+++ b/src/test/compile-fail/shadowed-use-visibility.rs
@@ -0,0 +1,26 @@
+// Copyright 2015 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 fn f() {}
+
+    use foo as bar;
+    pub use self::f as bar;
+}
+
+mod bar {
+    use foo::bar::f as g; //~ ERROR unresolved import
+
+    use foo as f;
+    pub use foo::*;
+}
+
+use bar::f::f; //~ ERROR unresolved import
+fn main() {}
diff --git a/src/test/run-pass/shadowed-use-visibility.rs b/src/test/run-pass/shadowed-use-visibility.rs
new file mode 100644
index 00000000000..d2a32da4fea
--- /dev/null
+++ b/src/test/run-pass/shadowed-use-visibility.rs
@@ -0,0 +1,20 @@
+// Copyright 2015 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 fn f() {}
+
+    pub use self::f as bar;
+    use foo as bar;
+}
+
+fn main() {
+    foo::bar();
+}