Auto merge of #52234 - petrochenkov:macuse2, r=Mark-Simulacrum

resolve: Modularize crate-local `#[macro_export] macro_rules`

Based on #50911, cc #50911 (comment)

`#[macro_export] macro_rules` items are collected from the whole crate and are planted into the root module as items, so the external view of the crate is symmetric with its internal view and something like `$crate::my_macro` where `my_macro` is `#[macro_export] macro_rules` works both locally and from other crates.

Closes #52726

Author: bors

src/librustc_resolve/build_reduced_graph.rs

@@ -59,7 +59,20 @@ impl<'a> ToNameBinding<'a> for (Module<'a>, ty::Visibility, Span, Mark) {
 impl<'a> ToNameBinding<'a> for (Def, ty::Visibility, Span, Mark) {
     fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> &'a NameBinding<'a> {
         arenas.alloc_name_binding(NameBinding {
-            kind: NameBindingKind::Def(self.0),
+            kind: NameBindingKind::Def(self.0, false),
+            vis: self.1,
+            span: self.2,
+            expansion: self.3,
+        })
+    }
+}
+
+pub(crate) struct IsMacroExport;
+
+impl<'a> ToNameBinding<'a> for (Def, ty::Visibility, Span, Mark, IsMacroExport) {
+    fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> &'a NameBinding<'a> {
+        arenas.alloc_name_binding(NameBinding {
+            kind: NameBindingKind::Def(self.0, true),
             vis: self.1,
             span: self.2,
             expansion: self.3,
@@ -772,6 +785,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
     fn visit_invoc(&mut self, id: ast::NodeId) -> &'b InvocationData<'b> {
         let mark = id.placeholder_to_mark();
         self.resolver.current_module.unresolved_invocations.borrow_mut().insert(mark);
+        self.resolver.unresolved_invocations_macro_export.insert(mark);
         let invocation = self.resolver.invocations[&mark];
         invocation.module.set(self.resolver.current_module);
         invocation.legacy_scope.set(self.legacy_scope);

src/librustc_resolve/lib.rs

@@ -1118,7 +1118,7 @@ impl<'a> ToNameBinding<'a> for &'a NameBinding<'a> {
 
 #[derive(Clone, Debug)]
 enum NameBindingKind<'a> {
-    Def(Def),
+    Def(Def, /* is_macro_export */ bool),
     Module(Module<'a>),
     Import {
         binding: &'a NameBinding<'a>,
@@ -1162,7 +1162,7 @@ impl<'a> NameBinding<'a> {
 
     fn def(&self) -> Def {
         match self.kind {
-            NameBindingKind::Def(def) => def,
+            NameBindingKind::Def(def, _) => def,
             NameBindingKind::Module(module) => module.def().unwrap(),
             NameBindingKind::Import { binding, .. } => binding.def(),
             NameBindingKind::Ambiguity { .. } => Def::Err,
@@ -1192,8 +1192,8 @@ impl<'a> NameBinding<'a> {
 
     fn is_variant(&self) -> bool {
         match self.kind {
-            NameBindingKind::Def(Def::Variant(..)) |
-            NameBindingKind::Def(Def::VariantCtor(..)) => true,
+            NameBindingKind::Def(Def::Variant(..), _) |
+            NameBindingKind::Def(Def::VariantCtor(..), _) => true,
             _ => false,
         }
     }
@@ -1242,7 +1242,7 @@ impl<'a> NameBinding<'a> {
 
     fn is_macro_def(&self) -> bool {
         match self.kind {
-            NameBindingKind::Def(Def::Macro(..)) => true,
+            NameBindingKind::Def(Def::Macro(..), _) => true,
             _ => false,
         }
     }
@@ -1398,7 +1398,7 @@ pub struct Resolver<'a> {
     macro_map: FxHashMap<DefId, Lrc<SyntaxExtension>>,
     macro_defs: FxHashMap<Mark, DefId>,
     local_macro_def_scopes: FxHashMap<NodeId, Module<'a>>,
-    macro_exports: Vec<Export>,
+    macro_exports: Vec<Export>, // FIXME: Remove when `use_extern_macros` is stabilized
     pub whitelisted_legacy_custom_derives: Vec<Name>,
     pub found_unresolved_macro: bool,
 
@@ -1428,6 +1428,9 @@ pub struct Resolver<'a> {
 
     /// Only supposed to be used by rustdoc, otherwise should be false.
     pub ignore_extern_prelude_feature: bool,
+
+    /// Macro invocations in the whole crate that can expand into a `#[macro_export] macro_rules`.
+    unresolved_invocations_macro_export: FxHashSet<Mark>,
 }
 
 /// Nothing really interesting here, it just provides memory for the rest of the crate.
@@ -1702,7 +1705,7 @@ impl<'a> Resolver<'a> {
 
             arenas,
             dummy_binding: arenas.alloc_name_binding(NameBinding {
-                kind: NameBindingKind::Def(Def::Err),
+                kind: NameBindingKind::Def(Def::Err, false),
                 expansion: Mark::root(),
                 span: DUMMY_SP,
                 vis: ty::Visibility::Public,
@@ -1732,6 +1735,7 @@ impl<'a> Resolver<'a> {
             current_type_ascription: Vec::new(),
             injected_crate: None,
             ignore_extern_prelude_feature: false,
+            unresolved_invocations_macro_export: FxHashSet(),
         }
     }
 

src/librustc_resolve/macros.rs

@@ -11,7 +11,7 @@
 use {AmbiguityError, CrateLint, Resolver, ResolutionError, resolve_error};
 use {Module, ModuleKind, NameBinding, NameBindingKind, PathResult};
 use Namespace::{self, MacroNS};
-use build_reduced_graph::BuildReducedGraphVisitor;
+use build_reduced_graph::{BuildReducedGraphVisitor, IsMacroExport};
 use resolve_imports::ImportResolver;
 use rustc::hir::def_id::{DefId, BUILTIN_MACROS_CRATE, CRATE_DEF_INDEX, DefIndex,
                          DefIndexAddressSpace};
@@ -193,7 +193,9 @@ impl<'a> base::Resolver for Resolver<'a> {
 
         self.current_module = invocation.module.get();
         self.current_module.unresolved_invocations.borrow_mut().remove(&mark);
+        self.unresolved_invocations_macro_export.remove(&mark);
         self.current_module.unresolved_invocations.borrow_mut().extend(derives);
+        self.unresolved_invocations_macro_export.extend(derives);
         for &derive in derives {
             self.invocations.insert(derive, invocation);
         }
@@ -215,7 +217,7 @@ impl<'a> base::Resolver for Resolver<'a> {
         let kind = ext.kind();
         self.macro_map.insert(def_id, ext);
         let binding = self.arenas.alloc_name_binding(NameBinding {
-            kind: NameBindingKind::Def(Def::Macro(def_id, kind)),
+            kind: NameBindingKind::Def(Def::Macro(def_id, kind), false),
             span: DUMMY_SP,
             vis: ty::Visibility::Invisible,
             expansion: Mark::root(),
@@ -711,12 +713,15 @@ impl<'a> Resolver<'a> {
 
             match (legacy_resolution, resolution) {
                 (Some(MacroBinding::Legacy(legacy_binding)), Ok(MacroBinding::Modern(binding))) => {
-                    let msg1 = format!("`{}` could refer to the macro defined here", ident);
-                    let msg2 = format!("`{}` could also refer to the macro imported here", ident);
-                    self.session.struct_span_err(span, &format!("`{}` is ambiguous", ident))
-                        .span_note(legacy_binding.span, &msg1)
-                        .span_note(binding.span, &msg2)
-                        .emit();
+                    if legacy_binding.def_id != binding.def_ignoring_ambiguity().def_id() {
+                        let msg1 = format!("`{}` could refer to the macro defined here", ident);
+                        let msg2 =
+                            format!("`{}` could also refer to the macro imported here", ident);
+                        self.session.struct_span_err(span, &format!("`{}` is ambiguous", ident))
+                            .span_note(legacy_binding.span, &msg1)
+                            .span_note(binding.span, &msg2)
+                            .emit();
+                    }
                 },
                 (None, Err(_)) => {
                     assert!(def.is_none());
@@ -850,12 +855,19 @@ impl<'a> Resolver<'a> {
             let def = Def::Macro(def_id, MacroKind::Bang);
             self.all_macros.insert(ident.name, def);
             if attr::contains_name(&item.attrs, "macro_export") {
-                self.macro_exports.push(Export {
-                    ident: ident.modern(),
-                    def: def,
-                    vis: ty::Visibility::Public,
-                    span: item.span,
-                });
+                if self.use_extern_macros {
+                    let module = self.graph_root;
+                    let vis = ty::Visibility::Public;
+                    self.define(module, ident, MacroNS,
+                                (def, vis, item.span, expansion, IsMacroExport));
+                } else {
+                    self.macro_exports.push(Export {
+                        ident: ident.modern(),
+                        def: def,
+                        vis: ty::Visibility::Public,
+                        span: item.span,
+                    });
+                }
             } else {
                 self.unused_macros.insert(def_id);
             }

src/librustc_resolve/resolve_imports.rs

@@ -211,7 +211,9 @@ impl<'a> Resolver<'a> {
         // if it cannot be shadowed by some new item/import expanded from a macro.
         // This happens either if there are no unexpanded macros, or expanded names cannot
         // shadow globs (that happens in macro namespace or with restricted shadowing).
-        let unexpanded_macros = !module.unresolved_invocations.borrow().is_empty();
+        let unexpanded_macros = !module.unresolved_invocations.borrow().is_empty() ||
+                                (ns == MacroNS && ptr::eq(module, self.graph_root) &&
+                                 !self.unresolved_invocations_macro_export.is_empty());
         if let Some(binding) = resolution.binding {
             if !unexpanded_macros || ns == MacroNS || restricted_shadowing {
                 return check_usable(self, binding);
@@ -363,6 +365,18 @@ impl<'a> Resolver<'a> {
                         resolution.binding = Some(binding);
                         resolution.shadowed_glob = Some(old_binding);
                     }
+                } else if let (&NameBindingKind::Def(_, true), &NameBindingKind::Def(_, true)) =
+                        (&old_binding.kind, &binding.kind) {
+
+                    this.session.buffer_lint_with_diagnostic(
+                        DUPLICATE_MACRO_EXPORTS,
+                        CRATE_NODE_ID,
+                        binding.span,
+                        &format!("a macro named `{}` has already been exported", ident),
+                        BuiltinLintDiagnostics::DuplicatedMacroExports(
+                            ident, old_binding.span, binding.span));
+
+                    resolution.binding = Some(binding);
                 } else {
                     return Err(old_binding);
                 }
@@ -766,7 +780,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
                                     match binding.kind {
                                         // Never suggest the name that has binding error
                                         // i.e. the name that cannot be previously resolved
-                                        NameBindingKind::Def(Def::Err) => return None,
+                                        NameBindingKind::Def(Def::Err, _) => return None,
                                         _ => Some(&i.name),
                                     }
                                 },

src/test/run-pass/auxiliary/issue_38715-modern.rs

@@ -0,0 +1,18 @@
+// Copyright 2016 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.
+
+#![feature(use_extern_macros)]
+#![allow(duplicate_macro_exports)]
+
+#[macro_export]
+macro_rules! foo_modern { ($i:ident) => {} }
+
+#[macro_export]
+macro_rules! foo_modern { () => {} }

src/test/run-pass/issue-38715.rs

@@ -9,12 +9,16 @@
 // except according to those terms.
 
 // aux-build:issue_38715.rs
+// aux-build:issue_38715-modern.rs
 
 // Test that `#[macro_export] macro_rules!` shadow earlier `#[macro_export] macro_rules!`
 
 #[macro_use]
 extern crate issue_38715;
+#[macro_use]
+extern crate issue_38715_modern;
 
 fn main() {
     foo!();
+    foo_modern!();
 }

src/test/ui/duplicate-check-macro-exports.rs

@@ -13,6 +13,6 @@
 pub use std::panic;
 
 #[macro_export]
-macro_rules! panic { () => {} } //~ ERROR a macro named `panic` has already been exported
+macro_rules! panic { () => {} } //~ ERROR the name `panic` is defined multiple times
 
 fn main() {}

src/test/ui/duplicate-check-macro-exports.stderr

@@ -1,14 +1,18 @@
-error: a macro named `panic` has already been exported
+error[E0255]: the name `panic` is defined multiple times
   --> $DIR/duplicate-check-macro-exports.rs:16:1
    |
-LL | macro_rules! panic { () => {} } //~ ERROR a macro named `panic` has already been exported
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `panic` already exported
+LL | pub use std::panic;
+   |         ---------- previous import of the macro `panic` here
+...
+LL | macro_rules! panic { () => {} } //~ ERROR the name `panic` is defined multiple times
+   | ^^^^^^^^^^^^^^^^^^ `panic` redefined here
    |
-note: previous macro export here
-  --> $DIR/duplicate-check-macro-exports.rs:13:9
+   = note: `panic` must be defined only once in the macro namespace of this module
+help: You can use `as` to change the binding name of the import
    |
-LL | pub use std::panic;
-   |         ^^^^^^^^^^
+LL | pub use std::panic as other_panic;
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 
+For more information about this error, try `rustc --explain E0255`.

src/test/ui/imports/local-modularized-tricky-fail-1.rs

@@ -0,0 +1,57 @@
+// 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.
+
+#![feature(decl_macro)]
+
+macro_rules! define_exported { () => {
+    #[macro_export]
+    macro_rules! exported {
+        () => ()
+    }
+}}
+macro_rules! define_panic { () => {
+    #[macro_export]
+    macro_rules! panic {
+        () => ()
+    }
+}}
+macro_rules! define_include { () => {
+    #[macro_export]
+    macro_rules! include {
+        () => ()
+    }
+}}
+
+use inner1::*;
+
+mod inner1 {
+    pub macro exported() {}
+}
+
+exported!(); //~ ERROR `exported` is ambiguous
+
+mod inner2 {
+    define_exported!();
+}
+
+fn main() {
+    panic!(); //~ ERROR `panic` is ambiguous
+              //~^ ERROR `panic` is ambiguous
+}
+
+mod inner3 {
+    define_panic!();
+}
+
+mod inner4 {
+    define_include!();
+}
+
+include!(); //~ ERROR `include` is ambiguous