Better support for import resolution in 3 namespaces

2 files changed, 107 insertions, 22 deletions

diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
index e2a7f5668d2..17aa510b565 100644
--- a/src/librustc_resolve/resolve_imports.rs
+++ b/src/librustc_resolve/resolve_imports.rs
@@ -33,7 +33,7 @@ use syntax::util::lev_distance::find_best_match_for_name;
 use syntax_pos::Span;
 
 use std::cell::{Cell, RefCell};
-use std::mem;
+use std::{mem, ptr};
 
 /// Contains data for specific types of import directives.
 #[derive(Clone, Debug)]
@@ -89,6 +89,8 @@ enum SingleImports<'a> {
     None,
     /// Only the given single import can define the name in the namespace.
     MaybeOne(&'a ImportDirective<'a>),
+    /// Only one of these two single imports can define the name in the namespace.
+    MaybeTwo(&'a ImportDirective<'a>, &'a ImportDirective<'a>),
     /// At least one single import will define the name in the namespace.
     AtLeastOne,
 }
@@ -101,21 +103,28 @@ impl<'a> Default for SingleImports<'a> {
 }
 
 impl<'a> SingleImports<'a> {
-    fn add_directive(&mut self, directive: &'a ImportDirective<'a>) {
+    fn add_directive(&mut self, directive: &'a ImportDirective<'a>, use_extern_macros: bool) {
         match *self {
             SingleImports::None => *self = SingleImports::MaybeOne(directive),
-            // If two single imports can define the name in the namespace, we can assume that at
-            // least one of them will define it since otherwise both would have to define only one
-            // namespace, leading to a duplicate error.
-            SingleImports::MaybeOne(_) => *self = SingleImports::AtLeastOne,
+            SingleImports::MaybeOne(directive_one) => *self = if use_extern_macros {
+                SingleImports::MaybeTwo(directive_one, directive)
+            } else {
+                SingleImports::AtLeastOne
+            },
+            // If three single imports can define the name in the namespace, we can assume that at
+            // least one of them will define it since otherwise we'd get duplicate errors in one of
+            // other namespaces.
+            SingleImports::MaybeTwo(..) => *self = SingleImports::AtLeastOne,
             SingleImports::AtLeastOne => {}
         };
     }
 
-    fn directive_failed(&mut self) {
+    fn directive_failed(&mut self, dir: &'a ImportDirective<'a>) {
         match *self {
             SingleImports::None => unreachable!(),
             SingleImports::MaybeOne(_) => *self = SingleImports::None,
+            SingleImports::MaybeTwo(dir1, dir2) =>
+                *self = SingleImports::MaybeOne(if ptr::eq(dir1, dir) { dir1 } else { dir2 }),
             SingleImports::AtLeastOne => {}
         }
     }
@@ -199,23 +208,50 @@ impl<'a> Resolver<'a> {
         }
 
         // Check if a single import can still define the name.
+        let resolve_single_import = |this: &mut Self, directive: &'a ImportDirective<'a>| {
+            let module = match directive.imported_module.get() {
+                Some(module) => module,
+                None => return false,
+            };
+            let ident = match directive.subclass {
+                SingleImport { source, .. } => source,
+                _ => unreachable!(),
+            };
+            match this.resolve_ident_in_module(module, ident, ns, false, false, path_span) {
+                Err(Determined) => {}
+                _ => return false,
+            }
+            true
+        };
         match resolution.single_imports {
             SingleImports::AtLeastOne => return Err(Undetermined),
-            SingleImports::MaybeOne(directive) if self.is_accessible(directive.vis.get()) => {
-                let module = match directive.imported_module.get() {
-                    Some(module) => module,
-                    None => return Err(Undetermined),
-                };
-                let ident = match directive.subclass {
-                    SingleImport { source, .. } => source,
-                    _ => unreachable!(),
-                };
-                match self.resolve_ident_in_module(module, ident, ns, false, false, path_span) {
-                    Err(Determined) => {}
-                    _ => return Err(Undetermined),
+            SingleImports::MaybeOne(directive) => {
+                let accessible = self.is_accessible(directive.vis.get());
+                if accessible {
+                    if !resolve_single_import(self, directive) {
+                        return Err(Undetermined)
+                    }
+                }
+            }
+            SingleImports::MaybeTwo(directive1, directive2) => {
+                let accessible1 = self.is_accessible(directive1.vis.get());
+                let accessible2 = self.is_accessible(directive2.vis.get());
+                if accessible1 && accessible2 {
+                    if !resolve_single_import(self, directive1) &&
+                       !resolve_single_import(self, directive2) {
+                        return Err(Undetermined)
+                    }
+                } else if accessible1 {
+                    if !resolve_single_import(self, directive1) {
+                        return Err(Undetermined)
+                    }
+                } else {
+                    if !resolve_single_import(self, directive2) {
+                        return Err(Undetermined)
+                    }
                 }
             }
-            SingleImports::MaybeOne(_) | SingleImports::None => {},
+            SingleImports::None => {},
         }
 
         let no_unresolved_invocations =
@@ -281,7 +317,7 @@ impl<'a> Resolver<'a> {
             SingleImport { target, .. } => {
                 self.per_ns(|this, ns| {
                     let mut resolution = this.resolution(current_module, target, ns).borrow_mut();
-                    resolution.single_imports.add_directive(directive);
+                    resolution.single_imports.add_directive(directive, this.use_extern_macros);
                 });
             }
             // We don't add prelude imports to the globs since they only affect lexical scopes,
@@ -575,7 +611,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
                 Err(Undetermined) => indeterminate = true,
                 Err(Determined) => {
                     this.update_resolution(parent, target, ns, |_, resolution| {
-                        resolution.single_imports.directive_failed()
+                        resolution.single_imports.directive_failed(directive)
                     });
                 }
                 Ok(binding) if !binding.is_importable() => {
diff --git a/src/test/ui/issue-50187.rs b/src/test/ui/issue-50187.rs
new file mode 100644
index 00000000000..87acf106393
--- /dev/null
+++ b/src/test/ui/issue-50187.rs
@@ -0,0 +1,49 @@
+// Copyright 2018 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.
+
+// compile-pass
+
+#![feature(use_extern_macros, decl_macro)]
+
+mod type_ns {
+    pub type A = u8;
+}
+mod value_ns {
+    pub const A: u8 = 0;
+}
+mod macro_ns {
+    pub macro A() {}
+}
+
+mod merge2 {
+    pub use type_ns::A;
+    pub use value_ns::A;
+}
+mod merge3 {
+    pub use type_ns::A;
+    pub use value_ns::A;
+    pub use macro_ns::A;
+}
+
+mod use2 {
+    pub use merge2::A;
+}
+mod use3 {
+    pub use merge3::A;
+}
+
+fn main() {
+    type B2 = use2::A;
+    let a2 = use2::A;
+
+    type B3 = use3::A;
+    let a3 = use3::A;
+    use3::A!();
+}