resolve: Merge resolution for `macro_rules` into the common early in-scope resolution function

`fn resolve_legacy_scope`/`fn resolve_lexical_macro_path_segment` -> `fn early_resolve_ident_in_lexical_scope`

6 files changed, 173 insertions, 253 deletions

diff --git a/src/librustc_resolve/Cargo.toml b/src/librustc_resolve/Cargo.toml
index 837340f70fc..3a8e84a3280 100644
--- a/src/librustc_resolve/Cargo.toml
+++ b/src/librustc_resolve/Cargo.toml
@@ -10,6 +10,7 @@ crate-type = ["dylib"]
 test = false
 
 [dependencies]
+bitflags = "1.0"
 log = "0.4"
 syntax = { path = "../libsyntax" }
 rustc = { path = "../librustc" }
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
index ece0188fa62..b41e9625e4e 100644
--- a/src/librustc_resolve/lib.rs
+++ b/src/librustc_resolve/lib.rs
@@ -18,6 +18,8 @@
 #![feature(slice_sort_by_cached_key)]
 
 #[macro_use]
+extern crate bitflags;
+#[macro_use]
 extern crate log;
 #[macro_use]
 extern crate syntax;
@@ -1210,10 +1212,6 @@ impl<'a> NameBinding<'a> {
         }
     }
 
-    fn get_macro<'b: 'a>(&self, resolver: &mut Resolver<'a, 'b>) -> Lrc<SyntaxExtension> {
-        resolver.get_macro(self.def_ignoring_ambiguity())
-    }
-
     // We sometimes need to treat variants as `pub` for backwards compatibility
     fn pseudo_vis(&self) -> ty::Visibility {
         if self.is_variant() && self.def().def_id().is_local() {
@@ -3664,8 +3662,8 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
                 self.resolve_ident_in_module(module, ident, ns, record_used, path_span)
             } else if opt_ns == Some(MacroNS) {
                 assert!(ns == TypeNS);
-                self.resolve_lexical_macro_path_segment(ident, ns, None, parent_scope, record_used,
-                                                        record_used, path_span).map(|(b, _)| b)
+                self.early_resolve_ident_in_lexical_scope(ident, ns, None, parent_scope,
+                                                          record_used, record_used, path_span)
             } else {
                 let record_used_id =
                     if record_used { crate_lint.node_id().or(Some(CRATE_NODE_ID)) } else { None };
diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs
index 2479c5161cb..6c067c669cb 100644
--- a/src/librustc_resolve/macros.rs
+++ b/src/librustc_resolve/macros.rs
@@ -43,9 +43,6 @@ use std::cell::Cell;
 use std::mem;
 use rustc_data_structures::sync::Lrc;
 
-#[derive(Clone, Copy)]
-crate struct FromPrelude(bool);
-
 #[derive(Clone)]
 pub struct InvocationData<'a> {
     def_index: DefIndex,
@@ -503,37 +500,36 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                     Err(Determinacy::Determined)
                 },
             };
+
             parent_scope.module.macro_resolutions.borrow_mut()
                 .push((path.into_boxed_slice(), span));
-            return def;
-        }
 
-        let result = if let Some(legacy_binding) = self.resolve_legacy_scope(path[0], Some(kind),
-                                                                             parent_scope, false) {
-            Ok(legacy_binding.def())
+            def
         } else {
-            match self.resolve_lexical_macro_path_segment(path[0], MacroNS, Some(kind),
-                                                          parent_scope, false, force, span) {
-                Ok((binding, _)) => Ok(binding.def_ignoring_ambiguity()),
+            let def = match self.early_resolve_ident_in_lexical_scope(path[0], MacroNS, Some(kind),
+                                                                      parent_scope, false, force,
+                                                                      span) {
+                Ok(binding) => Ok(binding.def_ignoring_ambiguity()),
                 Err(Determinacy::Undetermined) => return Err(Determinacy::Undetermined),
                 Err(Determinacy::Determined) => {
                     self.found_unresolved_macro = true;
                     Err(Determinacy::Determined)
                 }
-            }
-        };
+            };
 
-        parent_scope.module.legacy_macro_resolutions.borrow_mut()
-            .push((path[0], kind, parent_scope.clone(), result.ok()));
+            parent_scope.module.legacy_macro_resolutions.borrow_mut()
+                .push((path[0], kind, parent_scope.clone(), def.ok()));
 
-        result
+            def
+        }
     }
 
-    // Resolve the initial segment of a non-global macro path
-    // (e.g. `foo` in `foo::bar!(); or `foo!();`).
+    // Resolve an identifier in lexical scope.
     // This is a variation of `fn resolve_ident_in_lexical_scope` that can be run during
     // expansion and import resolution (perhaps they can be merged in the future).
-    crate fn resolve_lexical_macro_path_segment(
+    // The function is used for resolving initial segments of macro paths (e.g. `foo` in
+    // `foo::bar!(); or `foo!();`) and can be used for "uniform path" imports in the future.
+    crate fn early_resolve_ident_in_lexical_scope(
         &mut self,
         mut ident: Ident,
         ns: Namespace,
@@ -542,7 +538,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         record_used: bool,
         force: bool,
         path_span: Span,
-    ) -> Result<(&'a NameBinding<'a>, FromPrelude), Determinacy> {
+    ) -> Result<&'a NameBinding<'a>, Determinacy> {
         // General principles:
         // 1. Not controlled (user-defined) names should have higher priority than controlled names
         //    built into the language or standard library. This way we can add new names into the
@@ -565,21 +561,20 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         //    (open, not controlled).
         // 3. Standard library prelude (de-facto closed, controlled).
         // (Macro NS)
-        // 0. Derive helpers (open, not controlled). All ambiguities with other names
-        //    are currently reported as errors. They should be higher in priority than preludes
-        //    and probably even names in modules according to the "general principles" above. They
-        //    also should be subject to restricted shadowing because are effectively produced by
-        //    derives (you need to resolve the derive first to add helpers into scope), but they
-        //    should be available before the derive is expanded for compatibility.
-        //    It's mess in general, so we are being conservative for now.
-        // 1. Names in modules (both normal `mod`ules and blocks), loop through hygienic parents
-        //    (open, not controlled).
-        // 2. `macro_use` prelude (open, the open part is from macro expansions, not controlled).
-        // 2a. User-defined prelude from macro-use
+        // 1-2. `macro_rules` (open, not controlled), loop through legacy scopes. Have higher
+        //    priority than prelude macros, but create ambiguities with macros in modules.
+        // 1-2. Names in modules (both normal `mod`ules and blocks), loop through hygienic parents
+        //    (open, not controlled). Have higher priority than prelude macros, but create
+        //    ambiguities with `macro_rules`.
+        // 3. `macro_use` prelude (open, the open part is from macro expansions, not controlled).
+        // 3a. User-defined prelude from macro-use
         //    (open, the open part is from macro expansions, not controlled).
-        // 2b. Standard library prelude is currently implemented as `macro-use` (closed, controlled)
-        // 3. Language prelude: builtin macros (closed, controlled, except for legacy plugins).
-        // 4. Language prelude: builtin attributes (closed, controlled).
+        // 3b. Standard library prelude is currently implemented as `macro-use` (closed, controlled)
+        // 5. Language prelude: builtin macros (closed, controlled, except for legacy plugins).
+        // 6. Language prelude: builtin attributes (closed, controlled).
+        // 3-6. Legacy plugin helpers (open, not controlled). Similar to derive helpers,
+        //    but introduced by legacy plugins using `register_attribute`. Priority is somewhere
+        //    in prelude, not sure where exactly (creates ambiguities with any other prelude names).
         // N (unordered). Derive helpers (open, not controlled). All ambiguities with other names
         //    are currently reported as errors. They should be higher in priority than preludes
         //    and maybe even names in modules according to the "general principles" above. They
@@ -587,8 +582,29 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         //    derives (you need to resolve the derive first to add helpers into scope), but they
         //    should be available before the derive is expanded for compatibility.
         //    It's mess in general, so we are being conservative for now.
-        // N (unordered). Legacy plugin helpers (open, not controlled). Similar to derive helpers,
-        //    but introduced by legacy plugins using `register_attribute`.
+
+        enum WhereToResolve<'a> {
+            MacroRules(LegacyScope<'a>),
+            Module(Module<'a>),
+            MacroUsePrelude,
+            BuiltinMacros,
+            BuiltinAttrs,
+            DeriveHelpers,
+            LegacyPluginHelpers,
+            ExternPrelude,
+            ToolPrelude,
+            StdLibPrelude,
+            BuiltinTypes,
+        }
+
+        bitflags! {
+            struct Flags: u8 {
+                const DERIVE_HELPERS = 1 << 0;
+                const MACRO_RULES    = 1 << 1;
+                const MODULE         = 1 << 2;
+                const PRELUDE        = 1 << 3;
+            }
+        }
 
         assert!(force || !record_used); // `record_used` implies `force`
         ident = ident.modern();
@@ -604,26 +620,18 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         // }
         // So we have to save the innermost solution and continue searching in outer scopes
         // to detect potential ambiguities.
-        let mut innermost_result: Option<(&NameBinding, FromPrelude)> = None;
-
-        enum WhereToResolve<'a> {
-            Module(Module<'a>),
-            MacroUsePrelude,
-            BuiltinMacros,
-            BuiltinAttrs,
-            DeriveHelpers,
-            LegacyPluginHelpers,
-            ExternPrelude,
-            ToolPrelude,
-            StdLibPrelude,
-            BuiltinTypes,
-        }
+        let mut innermost_result: Option<(&NameBinding, Flags, /* conflicts with */ Flags)> = None;
 
         // Go through all the scopes and try to resolve the name.
-        let mut where_to_resolve = WhereToResolve::DeriveHelpers;
+        let mut where_to_resolve = WhereToResolve::MacroRules(parent_scope.legacy);
         let mut use_prelude = !parent_scope.module.no_implicit_prelude;
         loop {
             let result = match where_to_resolve {
+                WhereToResolve::MacroRules(legacy_scope) => match legacy_scope {
+                    LegacyScope::Binding(legacy_binding) if ident == legacy_binding.ident =>
+                        Ok((legacy_binding.binding, Flags::MACRO_RULES, Flags::MODULE)),
+                    _ => Err(Determinacy::Determined),
+                }
                 WhereToResolve::Module(module) => {
                     let orig_current_module = mem::replace(&mut self.current_module, module);
                     let binding = self.resolve_ident_in_module_unadjusted(
@@ -635,17 +643,17 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                         path_span,
                     );
                     self.current_module = orig_current_module;
-                    binding.map(|binding| (binding, FromPrelude(false)))
+                    binding.map(|binding| (binding, Flags::MODULE, Flags::empty()))
                 }
                 WhereToResolve::MacroUsePrelude => {
                     match self.macro_use_prelude.get(&ident.name).cloned() {
-                        Some(binding) => Ok((binding, FromPrelude(true))),
+                        Some(binding) => Ok((binding, Flags::PRELUDE, Flags::empty())),
                         None => Err(Determinacy::Determined),
                     }
                 }
                 WhereToResolve::BuiltinMacros => {
                     match self.builtin_macros.get(&ident.name).cloned() {
-                        Some(binding) => Ok((binding, FromPrelude(true))),
+                        Some(binding) => Ok((binding, Flags::PRELUDE, Flags::empty())),
                         None => Err(Determinacy::Determined),
                     }
                 }
@@ -654,7 +662,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                         let binding = (Def::NonMacroAttr(NonMacroAttrKind::Builtin),
                                        ty::Visibility::Public, ident.span, Mark::root())
                                        .to_name_binding(self.arenas);
-                        Ok((binding, FromPrelude(true)))
+                        Ok((binding, Flags::PRELUDE, Flags::empty()))
                     } else {
                         Err(Determinacy::Determined)
                     }
@@ -671,7 +679,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                                         (Def::NonMacroAttr(NonMacroAttrKind::DeriveHelper),
                                         ty::Visibility::Public, derive.span, Mark::root())
                                         .to_name_binding(self.arenas);
-                                    result = Ok((binding, FromPrelude(false)));
+                                    result = Ok((binding, Flags::DERIVE_HELPERS, Flags::all()));
                                     break;
                                 }
                             }
@@ -685,7 +693,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                         let binding = (Def::NonMacroAttr(NonMacroAttrKind::LegacyPluginHelper),
                                        ty::Visibility::Public, ident.span, Mark::root())
                                        .to_name_binding(self.arenas);
-                        Ok((binding, FromPrelude(false)))
+                        Ok((binding, Flags::PRELUDE, Flags::PRELUDE))
                     } else {
                         Err(Determinacy::Determined)
                     }
@@ -700,7 +708,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
 
                         let binding = (crate_root, ty::Visibility::Public,
                                        ident.span, Mark::root()).to_name_binding(self.arenas);
-                        Ok((binding, FromPrelude(true)))
+                        Ok((binding, Flags::PRELUDE, Flags::empty()))
                     } else {
                         Err(Determinacy::Determined)
                     }
@@ -709,7 +717,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                     if use_prelude && is_known_tool(ident.name) {
                         let binding = (Def::ToolMod, ty::Visibility::Public,
                                        ident.span, Mark::root()).to_name_binding(self.arenas);
-                        Ok((binding, FromPrelude(true)))
+                        Ok((binding, Flags::PRELUDE, Flags::empty()))
                     } else {
                         Err(Determinacy::Determined)
                     }
@@ -726,26 +734,34 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                                 false,
                                 path_span,
                             ) {
-                                result = Ok((binding, FromPrelude(true)));
+                                result = Ok((binding, Flags::PRELUDE, Flags::empty()));
                             }
                         }
                     }
                     result
                 }
                 WhereToResolve::BuiltinTypes => {
-                    if let Some(prim_ty) =
-                            self.primitive_type_table.primitive_types.get(&ident.name).cloned() {
-                        let binding = (Def::PrimTy(prim_ty), ty::Visibility::Public,
-                                       ident.span, Mark::root()).to_name_binding(self.arenas);
-                        Ok((binding, FromPrelude(true)))
-                    } else {
-                        Err(Determinacy::Determined)
+                    match self.primitive_type_table.primitive_types.get(&ident.name).cloned() {
+                        Some(prim_ty) => {
+                            let binding = (Def::PrimTy(prim_ty), ty::Visibility::Public,
+                                           ident.span, Mark::root()).to_name_binding(self.arenas);
+                            Ok((binding, Flags::PRELUDE, Flags::empty()))
+                        }
+                        None => Err(Determinacy::Determined)
                     }
                 }
             };
 
             macro_rules! continue_search { () => {
                 where_to_resolve = match where_to_resolve {
+                    WhereToResolve::MacroRules(legacy_scope) => match legacy_scope {
+                        LegacyScope::Binding(binding) =>
+                            WhereToResolve::MacroRules(binding.parent_legacy_scope),
+                        LegacyScope::Invocation(invocation) =>
+                            WhereToResolve::MacroRules(invocation.output_legacy_scope.get()),
+                        LegacyScope::Empty => WhereToResolve::Module(parent_scope.module),
+                        LegacyScope::Uninitialized => unreachable!(),
+                    }
                     WhereToResolve::Module(module) => {
                         match self.hygienic_lexical_parent(module, &mut ident.span) {
                             Some(parent_module) => WhereToResolve::Module(parent_module),
@@ -778,36 +794,33 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
             }}
 
             match result {
-                Ok(result) => {
-                    if sub_namespace_mismatch(kind, result.0.macro_kind()) {
+                Ok((binding, flags, ambig_flags)) => {
+                    if sub_namespace_mismatch(kind, binding.macro_kind()) {
                         continue_search!();
                     }
 
                     if !record_used {
-                        return Ok(result);
+                        return Ok(binding);
                     }
 
-                    if let Some(innermost_result) = innermost_result {
+                    if let Some((innermost_binding, innermost_flags, innermost_ambig_flags))
+                            = innermost_result {
                         // Found another solution, if the first one was "weak", report an error.
-                        let prohibit_ambiguities = |def| {
-                            def == Def::NonMacroAttr(NonMacroAttrKind::DeriveHelper) ||
-                            def == Def::NonMacroAttr(NonMacroAttrKind::LegacyPluginHelper)
-                        };
-                        if result.0.def() != innermost_result.0.def() &&
-                           (innermost_result.0.is_glob_import() ||
-                            innermost_result.0.may_appear_after(parent_scope.expansion, result.0) ||
-                            prohibit_ambiguities(innermost_result.0.def()) ||
-                            prohibit_ambiguities(result.0.def())) {
+                        if binding.def() != innermost_binding.def() &&
+                           (innermost_binding.is_glob_import() ||
+                            innermost_binding.may_appear_after(parent_scope.expansion, binding) ||
+                            innermost_flags.intersects(ambig_flags) ||
+                            flags.intersects(innermost_ambig_flags)) {
                             self.ambiguity_errors.push(AmbiguityError {
                                 ident,
-                                b1: innermost_result.0,
-                                b2: result.0,
+                                b1: innermost_binding,
+                                b2: binding,
                             });
-                            return Ok(innermost_result);
+                            return Ok(innermost_binding);
                         }
                     } else {
                         // Found the first solution.
-                        innermost_result = Some(result);
+                        innermost_result = Some((binding, flags, ambig_flags));
                     }
 
                     continue_search!();
@@ -820,8 +833,8 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
         }
 
         // The first found solution was the only one, return it.
-        if let Some(innermost_result) = innermost_result {
-            return Ok(innermost_result);
+        if let Some((binding, ..)) = innermost_result {
+            return Ok(binding);
         }
 
         let determinacy = Determinacy::determined(force);
@@ -833,92 +846,12 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
             let binding = (Def::NonMacroAttr(NonMacroAttrKind::Custom),
                            ty::Visibility::Public, ident.span, Mark::root())
                            .to_name_binding(self.arenas);
-            Ok((binding, FromPrelude(true)))
+            Ok(binding)
         } else {
             Err(determinacy)
         }
     }
 
-    fn resolve_legacy_scope(
-        &mut self,
-        ident: Ident,
-        kind: Option<MacroKind>,
-        parent_scope: &ParentScope<'a>,
-        record_used: bool,
-    ) -> Option<&'a NameBinding<'a>> {
-        if sub_namespace_mismatch(kind, Some(MacroKind::Bang)) {
-            return None;
-        }
-
-        let ident = ident.modern();
-
-        // This is *the* result, resolution from the scope closest to the resolved identifier.
-        // However, sometimes this result is "weak" because it comes from a macro expansion,
-        // and in this case it cannot shadow names from outer scopes, e.g.
-        // macro_rules! m { ... } // solution in outer scope
-        // {
-        //     define_m!(); // generates another `macro_rules! m` - innermost solution
-        //                  // weak, cannot shadow the outer `m`, need to report ambiguity error
-        //     m!();
-        // }
-        // So we have to save the innermost solution and continue searching in outer scopes
-        // to detect potential ambiguities.
-        let mut innermost_result: Option<&NameBinding> = None;
-
-        // Go through all the scopes and try to resolve the name.
-        let mut where_to_resolve = parent_scope.legacy;
-        loop {
-            let result = match where_to_resolve {
-                LegacyScope::Binding(legacy_binding) if ident == legacy_binding.ident =>
-                    Some(legacy_binding.binding),
-                _ => None,
-            };
-
-            macro_rules! continue_search { () => {
-                where_to_resolve = match where_to_resolve {
-                    LegacyScope::Empty => break, // nowhere else to search
-                    LegacyScope::Binding(binding) => binding.parent_legacy_scope,
-                    LegacyScope::Invocation(invocation) => invocation.output_legacy_scope.get(),
-                    LegacyScope::Uninitialized => unreachable!(),
-                };
-
-                continue;
-            }}
-
-            match result {
-                Some(result) => {
-                    if !record_used {
-                        return Some(result);
-                    }
-
-                    if let Some(innermost_result) = innermost_result {
-                        // Found another solution, if the first one was "weak", report an error.
-                        if result.def() != innermost_result.def() &&
-                           innermost_result.may_appear_after(parent_scope.expansion, result) {
-                            self.ambiguity_errors.push(AmbiguityError {
-                                ident,
-                                b1: innermost_result,
-                                b2: result,
-                            });
-                            return Some(innermost_result);
-                        }
-                    } else {
-                        // Found the first solution.
-                        innermost_result = Some(result);
-                    }
-
-                    continue_search!();
-                }
-                None => {
-                    continue_search!();
-                }
-            }
-        }
-
-        // The first found solution was the only one (or there was no solution at all), return it.
-        innermost_result
-    }
-
     pub fn finalize_current_module_macro_resolutions(&mut self) {
         let module = self.current_module;
         for &(ref path, span) in module.macro_resolutions.borrow().iter() {
@@ -933,80 +866,51 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
 
         let legacy_macro_resolutions =
             mem::replace(&mut *module.legacy_macro_resolutions.borrow_mut(), Vec::new());
-        for (ident, kind, parent_scope, def) in legacy_macro_resolutions {
-            let span = ident.span;
-            let legacy_resolution = self.resolve_legacy_scope(
-                ident, Some(kind), &parent_scope, true
+        for (ident, kind, parent_scope, initial_def) in legacy_macro_resolutions {
+            let binding = self.early_resolve_ident_in_lexical_scope(
+                ident, MacroNS, Some(kind), &parent_scope, true, true, ident.span
             );
-            let resolution = self.resolve_lexical_macro_path_segment(
-                ident, MacroNS, Some(kind), &parent_scope, true, true, span
-            );
-
-            let check_consistency = |this: &Self, new_def: Def| {
-                if let Some(def) = def {
-                    if this.ambiguity_errors.is_empty() && new_def != def && new_def != Def::Err {
-                        // Make sure compilation does not succeed if preferred macro resolution
-                        // has changed after the macro had been expanded. In theory all such
-                        // situations should be reported as ambiguity errors, so this is span-bug.
-                        span_bug!(span, "inconsistent resolution for a macro");
+            match binding {
+                Ok(binding) => {
+                    self.record_use(ident, MacroNS, binding);
+                    let def = binding.def_ignoring_ambiguity();
+                    if let Some(initial_def) = initial_def {
+                        if self.ambiguity_errors.is_empty() &&
+                           def != initial_def && def != Def::Err {
+                            // Make sure compilation does not succeed if preferred macro resolution
+                            // has changed after the macro had been expanded. In theory all such
+                            // situations should be reported as ambiguity errors, so this is a bug.
+                            span_bug!(ident.span, "inconsistent resolution for a macro");
+                        }
+                    } else {
+                        // It's possible that the macro was unresolved (indeterminate) and silently
+                        // expanded into a dummy fragment for recovery during expansion.
+                        // Now, post-expansion, the resolution may succeed, but we can't change the
+                        // past and need to report an error.
+                        let msg = format!("cannot determine resolution for the {} `{}`",
+                                          kind.descr(), ident);
+                        let msg_note = "import resolution is stuck, try simplifying macro imports";
+                        self.session.struct_span_err(ident.span, &msg).note(msg_note).emit();
                     }
-                } else {
-                    // It's possible that the macro was unresolved (indeterminate) and silently
-                    // expanded into a dummy fragment for recovery during expansion.
-                    // Now, post-expansion, the resolution may succeed, but we can't change the
-                    // past and need to report an error.
-                    let msg =
-                        format!("cannot determine resolution for the {} `{}`", kind.descr(), ident);
-                    let msg_note = "import resolution is stuck, try simplifying macro imports";
-                    this.session.struct_span_err(span, &msg).note(msg_note).emit();
                 }
-            };
-
-            match (legacy_resolution, resolution) {
-                (None, Err(_)) => {
-                    assert!(def.is_none());
+                Err(..) => {
+                    assert!(initial_def.is_none());
                     let bang = if kind == MacroKind::Bang { "!" } else { "" };
                     let msg =
                         format!("cannot find {} `{}{}` in this scope", kind.descr(), ident, bang);
-                    let mut err = self.session.struct_span_err(span, &msg);
-                    self.suggest_macro_name(&ident.as_str(), kind, &mut err, span);
+                    let mut err = self.session.struct_span_err(ident.span, &msg);
+                    self.suggest_macro_name(&ident.as_str(), kind, &mut err, ident.span);
                     err.emit();
-                },
-                (Some(legacy_binding), Ok((binding, FromPrelude(from_prelude))))
-                        if legacy_binding.def() != binding.def_ignoring_ambiguity() &&
-                           (!from_prelude &&
-                            !self.disambiguate_legacy_vs_modern(legacy_binding, binding) ||
-                            legacy_binding.may_appear_after(parent_scope.expansion, binding)) => {
-                    self.report_ambiguity_error(ident, legacy_binding, binding);
-                },
-                // OK, non-macro-expanded legacy wins over prelude even if defs are different
-                // Also, non-macro-expanded legacy wins over modern from the same module
-                // Also, legacy and modern can co-exist if their defs are same
-                (Some(legacy_binding), Ok(_)) |
-                // OK, unambiguous resolution
-                (Some(legacy_binding), Err(_)) => {
-                    check_consistency(self, legacy_binding.def());
                 }
-                // OK, unambiguous resolution
-                (None, Ok((binding, FromPrelude(from_prelude)))) => {
-                    check_consistency(self, binding.def_ignoring_ambiguity());
-                    if from_prelude {
-                        self.record_use(ident, MacroNS, binding);
-                    }
-                }
-            };
+            }
         }
 
         let builtin_attrs = mem::replace(&mut *module.builtin_attrs.borrow_mut(), Vec::new());
         for (ident, parent_scope) in builtin_attrs {
-            let resolve_legacy = |this: &mut Self| this.resolve_legacy_scope(
-                ident, Some(MacroKind::Attr), &parent_scope, true
-            );
-            let resolve_modern = |this: &mut Self| this.resolve_lexical_macro_path_segment(
+            let binding = self.early_resolve_ident_in_lexical_scope(
                 ident, MacroNS, Some(MacroKind::Attr), &parent_scope, true, true, ident.span
-            ).map(|(binding, _)| binding).ok();
-
-            if let Some(binding) = resolve_legacy(self).or_else(|| resolve_modern(self)) {
+            );
+            if let Ok(binding) = binding {
                 if binding.def_ignoring_ambiguity() !=
                         Def::NonMacroAttr(NonMacroAttrKind::Builtin) {
                     let builtin_binding = (Def::NonMacroAttr(NonMacroAttrKind::Builtin),
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
index 33313139815..6e9877b1ab6 100644
--- a/src/librustc_resolve/resolve_imports.rs
+++ b/src/librustc_resolve/resolve_imports.rs
@@ -334,7 +334,7 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
         // expansion. With restricted shadowing names from globs and macro expansions cannot
         // shadow names from outer scopes, so we can freely fallback from module search to search
         // in outer scopes. To continue search in outer scopes we have to lie a bit and return
-        // `Determined` to `resolve_lexical_macro_path_segment` even if the correct answer
+        // `Determined` to `early_resolve_ident_in_lexical_scope` even if the correct answer
         // for in-module resolution could be `Undetermined`.
         if restricted_shadowing {
             return Err(Determined);
diff --git a/src/test/ui/imports/macros.stderr b/src/test/ui/imports/macros.stderr
index 209d449dfd8..965b23e1a5c 100644
--- a/src/test/ui/imports/macros.stderr
+++ b/src/test/ui/imports/macros.stderr
@@ -34,6 +34,23 @@ LL |     use two_macros::m;
    |         ^^^^^^^^^^^^^
    = note: macro-expanded macro imports do not shadow
 
-error: aborting due to 2 previous errors
+error[E0659]: `m` is ambiguous
+  --> $DIR/macros.rs:48:5
+   |
+LL |     m!(); //~ ERROR ambiguous
+   |     ^ ambiguous name
+   |
+note: `m` could refer to the name defined here
+  --> $DIR/macros.rs:46:5
+   |
+LL |     macro_rules! m { () => {} }
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: `m` could also refer to the name imported here
+  --> $DIR/macros.rs:47:9
+   |
+LL |     use two_macros::m;
+   |         ^^^^^^^^^^^^^
+
+error: aborting due to 3 previous errors
 
 For more information about this error, try `rustc --explain E0659`.
diff --git a/src/test/ui/imports/shadow_builtin_macros.stderr b/src/test/ui/imports/shadow_builtin_macros.stderr
index e554bbb4f31..7e5ab0c5abe 100644
--- a/src/test/ui/imports/shadow_builtin_macros.stderr
+++ b/src/test/ui/imports/shadow_builtin_macros.stderr
@@ -1,21 +1,4 @@
 error[E0659]: `panic` is ambiguous
-  --> $DIR/shadow_builtin_macros.rs:43:5
-   |
-LL |     panic!(); //~ ERROR `panic` is ambiguous
-   |     ^^^^^ ambiguous name
-   |
-note: `panic` could refer to the name defined here
-  --> $DIR/shadow_builtin_macros.rs:40:9
-   |
-LL |         macro_rules! panic { () => {} }
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-LL |     } }
-LL |     m!();
-   |     ----- in this macro invocation
-   = note: `panic` is also a builtin macro
-   = note: macro-expanded macros do not shadow
-
-error[E0659]: `panic` is ambiguous
   --> $DIR/shadow_builtin_macros.rs:25:14
    |
 LL |     fn f() { panic!(); } //~ ERROR ambiguous
@@ -43,6 +26,23 @@ LL |     ::two_macros::m!(use foo::panic;);
    = note: `panic` is also a builtin macro
    = note: macro-expanded macro imports do not shadow
 
+error[E0659]: `panic` is ambiguous
+  --> $DIR/shadow_builtin_macros.rs:43:5
+   |
+LL |     panic!(); //~ ERROR `panic` is ambiguous
+   |     ^^^^^ ambiguous name
+   |
+note: `panic` could refer to the name defined here
+  --> $DIR/shadow_builtin_macros.rs:40:9
+   |
+LL |         macro_rules! panic { () => {} }
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     } }
+LL |     m!();
+   |     ----- in this macro invocation
+   = note: `panic` is also a builtin macro
+   = note: macro-expanded macros do not shadow
+
 error[E0659]: `n` is ambiguous
   --> $DIR/shadow_builtin_macros.rs:59:5
    |