item_like_imports: Allow unused ambiguous glob imports.

Author: jseyfried

src/librustc_resolve/lib.rs

@@ -874,6 +874,10 @@ enum NameBindingKind<'a> {
         binding: &'a NameBinding<'a>,
         directive: &'a ImportDirective<'a>,
     },
+    Ambiguity {
+        b1: &'a NameBinding<'a>,
+        b2: &'a NameBinding<'a>,
+    }
 }
 
 #[derive(Clone, Debug)]
@@ -885,6 +889,7 @@ impl<'a> NameBinding<'a> {
             NameBindingKind::Module(module) => Some(module),
             NameBindingKind::Def(_) => None,
             NameBindingKind::Import { binding, .. } => binding.module(),
+            NameBindingKind::Ambiguity { ..  } => None,
         }
     }
 
@@ -893,6 +898,7 @@ impl<'a> NameBinding<'a> {
             NameBindingKind::Def(def) => def,
             NameBindingKind::Module(module) => module.def.unwrap(),
             NameBindingKind::Import { binding, .. } => binding.def(),
+            NameBindingKind::Ambiguity { .. } => Def::Err,
         }
     }
 
@@ -922,6 +928,7 @@ impl<'a> NameBinding<'a> {
     fn is_glob_import(&self) -> bool {
         match self.kind {
             NameBindingKind::Import { directive, .. } => directive.is_glob(),
+            NameBindingKind::Ambiguity { .. } => true,
             _ => false,
         }
     }
@@ -932,6 +939,14 @@ impl<'a> NameBinding<'a> {
             _ => true,
         }
     }
+
+    fn ambiguity(&self) -> Option<(&'a NameBinding<'a>, &'a NameBinding<'a>)> {
+        match self.kind {
+            NameBindingKind::Ambiguity { b1, b2 } => Some((b1, b2)),
+            NameBindingKind::Import { binding, .. } => binding.ambiguity(),
+            _ => None,
+        }
+    }
 }
 
 /// Interns the names of the primitive types.
@@ -1249,7 +1264,8 @@ impl<'a> Resolver<'a> {
         match ns { ValueNS => &mut self.value_ribs, TypeNS => &mut self.type_ribs }
     }
 
-    fn record_use(&mut self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>) {
+    fn record_use(&mut self, name: Name, ns: Namespace, binding: &'a NameBinding<'a>, span: Span)
+                  -> bool /* true if an error was reported */ {
         // track extern crates for unused_extern_crate lint
         if let Some(DefId { krate, .. }) = binding.module().and_then(ModuleS::def_id) {
             self.used_crates.insert(krate);
@@ -1259,6 +1275,19 @@ impl<'a> Resolver<'a> {
             self.used_imports.insert((directive.id, ns));
             self.add_to_glob_map(directive.id, name);
         }
+
+        if let Some((b1, b2)) = binding.ambiguity() {
+            let msg1 = format!("`{}` could resolve to the name imported here", name);
+            let msg2 = format!("`{}` could also resolve to the name imported here", name);
+            self.session.struct_span_err(span, &format!("`{}` is ambiguous", name))
+                .span_note(b1.span, &msg1)
+                .span_note(b2.span, &msg2)
+                .note(&format!("Consider adding an explicit import of `{}` to disambiguate", name))
+                .emit();
+            return true;
+        }
+
+        false
     }
 
     fn add_to_glob_map(&mut self, id: NodeId, name: Name) {
@@ -2294,7 +2323,8 @@ impl<'a> Resolver<'a> {
                             Def::Struct(..) | Def::Variant(..) |
                             Def::Const(..) | Def::AssociatedConst(..) if !always_binding => {
                                 // A constant, unit variant, etc pattern.
-                                self.record_use(ident.node.name, ValueNS, binding.unwrap());
+                                let name = ident.node.name;
+                                self.record_use(name, ValueNS, binding.unwrap(), ident.span);
                                 Some(PathResolution::new(def))
                             }
                             Def::Struct(..) | Def::Variant(..) |

src/librustc_resolve/resolve_imports.rs

@@ -181,7 +181,9 @@ impl<'a> Resolver<'a> {
                 if is_disallowed_private_import(binding) {
                     return Failed(None);
                 }
-                self.record_use(name, ns, binding);
+                if self.record_use(name, ns, binding, span) {
+                    return Success(self.dummy_binding);
+                }
                 if !self.is_accessible(binding.vis) {
                     self.privacy_errors.push(PrivacyError(span, name, binding));
                 }
@@ -323,7 +325,18 @@ impl<'a> Resolver<'a> {
                     if !this.new_import_semantics || !old_binding.is_glob_import() {
                         resolution.duplicate_globs.push(binding);
                     } else if binding.def() != old_binding.def() {
-                        resolution.duplicate_globs.push(binding);
+                        resolution.binding = Some(this.arenas.alloc_name_binding(NameBinding {
+                            kind: NameBindingKind::Ambiguity {
+                                b1: old_binding,
+                                b2: binding,
+                            },
+                            vis: if old_binding.vis.is_at_least(binding.vis, this) {
+                                old_binding.vis
+                            } else {
+                                binding.vis
+                            },
+                            span: old_binding.span,
+                        }));
                     } else if !old_binding.vis.is_at_least(binding.vis, this) {
                         // We are glob-importing the same item but with greater visibility.
                         resolution.binding = Some(binding);
@@ -597,7 +610,10 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
 
         for &(ns, result) in &[(ValueNS, value_result), (TypeNS, type_result)] {
             if let Ok(binding) = result {
-                self.record_use(name, ns, binding);
+                if self.record_use(name, ns, binding, directive.span) {
+                    self.resolution(module, name, ns).borrow_mut().binding =
+                        Some(self.dummy_binding);
+                }
             }
         }
 
@@ -759,17 +775,16 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
                         }
                     }
 
-                    self.report_conflict(module, name, ns, duplicate_glob, binding);
-                }
-            } else if binding.is_glob_import() {
-                for duplicate_glob in resolution.duplicate_globs.iter() {
                     self.report_conflict(module, name, ns, duplicate_glob, binding);
                 }
             }
 
             if binding.vis == ty::Visibility::Public &&
                (binding.is_import() || binding.is_extern_crate()) {
-                reexports.push(Export { name: name, def_id: binding.def().def_id() });
+                let def = binding.def();
+                if def != Def::Err {
+                    reexports.push(Export { name: name, def_id: def.def_id() });
+                }
             }
 
             if let NameBindingKind::Import { binding: orig_binding, directive, .. } = binding.kind {

src/test/compile-fail/imports/duplicate.rs

@@ -33,6 +33,20 @@ mod e {
     pub use c::*; // ok
 }
 
+mod f {
+    pub use a::*; //~ NOTE `foo` could resolve to the name imported here
+    pub use b::*; //~ NOTE `foo` could also resolve to the name imported here
+}
+
+mod g {
+    pub use a::*; //~ NOTE `foo` could resolve to the name imported here
+    pub use f::*; //~ NOTE `foo` could also resolve to the name imported here
+}
+
 fn main() {
     e::foo();
+    f::foo(); //~ ERROR `foo` is ambiguous
+              //~| NOTE Consider adding an explicit import of `foo` to disambiguate
+    g::foo(); //~ ERROR `foo` is ambiguous
+              //~| NOTE Consider adding an explicit import of `foo` to disambiguate
 }

src/test/run-pass/imports.rs

@@ -63,4 +63,14 @@ mod c {
     }
 }
 
+// Unused names can be ambiguous.
+mod d {
+    pub use foo::*; // This imports `f` in the value namespace.
+    pub use bar::*; // This also imports `f` in the value namespace.
+}
+
+mod e {
+    pub use d::*; // n.b. Since `e::f` is not used, this is not considered to be a use of `d::f`.
+}
+
 fn main() {}