Auto merge of #50355 - petrochenkov:50187, r=oli-obk

Fix an unresolved import issue with enabled `use_extern_macros`

This is a kinda ugly special-purpose solution that will break if we suddenly add a fourth namespace, but I hope to come up with something more general if I get to import resolution refactoring this summer.

Fixes https://github.com/rust-lang/rust/issues/50187 thus removing a blocker for stabilization of `use_extern_macros`

4 files changed, 122 insertions, 36 deletions

diff --git a/src/librustc_resolve/build_reduced_graph.rs b/src/librustc_resolve/build_reduced_graph.rs
index e5e6c22c7a2..45fd82b33bd 100644
--- a/src/librustc_resolve/build_reduced_graph.rs
+++ b/src/librustc_resolve/build_reduced_graph.rs
@@ -17,7 +17,7 @@ use macros::{InvocationData, LegacyScope};
 use resolve_imports::ImportDirective;
 use resolve_imports::ImportDirectiveSubclass::{self, GlobImport, SingleImport};
 use {Module, ModuleData, ModuleKind, NameBinding, NameBindingKind, ToNameBinding};
-use {Resolver, ResolverArenas};
+use {PerNS, Resolver, ResolverArenas};
 use Namespace::{self, TypeNS, ValueNS, MacroNS};
 use {resolve_error, resolve_struct_error, ResolutionError};
 
@@ -174,7 +174,11 @@ impl<'a> Resolver<'a> {
                 let subclass = SingleImport {
                     target: ident,
                     source,
-                    result: self.per_ns(|_, _| Cell::new(Err(Undetermined))),
+                    result: PerNS {
+                        type_ns: Cell::new(Err(Undetermined)),
+                        value_ns: Cell::new(Err(Undetermined)),
+                        macro_ns: Cell::new(Err(Undetermined)),
+                    },
                     type_ns_only,
                 };
                 self.add_import_directive(
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index 26c0ede48b7..d4b212a15d8 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -700,7 +700,7 @@ pub enum Namespace {
 pub struct PerNS<T> {
     value_ns: T,
     type_ns: T,
-    macro_ns: Option<T>,
+    macro_ns: T,
 }
 
 impl<T> ::std::ops::Index<Namespace> for PerNS<T> {
@@ -709,7 +709,7 @@ impl<T> ::std::ops::Index<Namespace> for PerNS<T> {
         match ns {
             ValueNS => &self.value_ns,
             TypeNS => &self.type_ns,
-            MacroNS => self.macro_ns.as_ref().unwrap(),
+            MacroNS => &self.macro_ns,
         }
     }
 }
@@ -719,7 +719,7 @@ impl<T> ::std::ops::IndexMut<Namespace> for PerNS<T> {
         match ns {
             ValueNS => &mut self.value_ns,
             TypeNS => &mut self.type_ns,
-            MacroNS => self.macro_ns.as_mut().unwrap(),
+            MacroNS => &mut self.macro_ns,
         }
     }
 }
@@ -1728,7 +1728,7 @@ impl<'a> Resolver<'a> {
             ribs: PerNS {
                 value_ns: vec![Rib::new(ModuleRibKind(graph_root))],
                 type_ns: vec![Rib::new(ModuleRibKind(graph_root))],
-                macro_ns: Some(vec![Rib::new(ModuleRibKind(graph_root))]),
+                macro_ns: vec![Rib::new(ModuleRibKind(graph_root))],
             },
             label_ribs: Vec::new(),
 
@@ -1808,14 +1808,11 @@ impl<'a> Resolver<'a> {
     }
 
     /// Runs the function on each namespace.
-    fn per_ns<T, F: FnMut(&mut Self, Namespace) -> T>(&mut self, mut f: F) -> PerNS<T> {
-        PerNS {
-            type_ns: f(self, TypeNS),
-            value_ns: f(self, ValueNS),
-            macro_ns: match self.use_extern_macros {
-                true => Some(f(self, MacroNS)),
-                false => None,
-            },
+    fn per_ns<F: FnMut(&mut Self, Namespace)>(&mut self, mut f: F) {
+        f(self, TypeNS);
+        f(self, ValueNS);
+        if self.use_extern_macros {
+            f(self, MacroNS);
         }
     }
 
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!();
+}