Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
18 changes: 11 additions & 7 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1117,7 +1117,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>,
Expand Down Expand Up @@ -1161,7 +1161,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,
Expand Down Expand Up @@ -1191,8 +1191,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,
}
}
Expand Down Expand Up @@ -1241,7 +1241,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,
}
}
Expand Down Expand Up @@ -1397,7 +1397,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,

Expand Down Expand Up @@ -1427,6 +1427,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.
Expand Down Expand Up @@ -1701,7 +1704,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,
Expand Down Expand Up @@ -1731,6 +1734,7 @@ impl<'a> Resolver<'a> {
current_type_ascription: Vec::new(),
injected_crate: None,
ignore_extern_prelude_feature: false,
unresolved_invocations_macro_export: FxHashSet(),
}
}

Expand Down
40 changes: 26 additions & 14 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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);
}
Expand All @@ -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(),
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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);
}
Expand Down
18 changes: 16 additions & 2 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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),
}
},
Expand Down
18 changes: 18 additions & 0 deletions src/test/run-pass/auxiliary/issue_38715-modern.rs
Original file line number Diff line number Diff line change
@@ -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 { () => {} }
4 changes: 4 additions & 0 deletions src/test/run-pass/issue-38715.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!();
}
2 changes: 1 addition & 1 deletion src/test/ui/duplicate-check-macro-exports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
18 changes: 11 additions & 7 deletions src/test/ui/duplicate-check-macro-exports.stderr
Original file line number Diff line number Diff line change
@@ -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`.
57 changes: 57 additions & 0 deletions src/test/ui/imports/local-modularized-tricky-fail-1.rs
Original file line number Diff line number Diff line change
@@ -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
Loading