diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index b4e7f3a8b746c..e97d8efe800a4 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -72,11 +72,10 @@ use syntax_pos::{Span, DUMMY_SP, MultiSpan}; use errors::{Applicability, DiagnosticBuilder, DiagnosticId}; use std::cell::{Cell, RefCell}; -use std::cmp; +use std::{cmp, fmt, iter, ptr}; use std::collections::BTreeSet; -use std::fmt; -use std::iter; use std::mem::replace; +use rustc_data_structures::ptr_key::PtrKey; use rustc_data_structures::sync::Lrc; use resolve_imports::{ImportDirective, ImportDirectiveSubclass, NameResolution, ImportResolver}; @@ -1114,6 +1113,17 @@ impl<'a> ModuleData<'a> { fn nearest_item_scope(&'a self) -> Module<'a> { if self.is_trait() { self.parent.unwrap() } else { self } } + + fn is_ancestor_of(&self, mut other: &Self) -> bool { + while !ptr::eq(self, other) { + if let Some(parent) = other.parent { + other = parent; + } else { + return false; + } + } + true + } } impl<'a> fmt::Debug for ModuleData<'a> { @@ -1411,6 +1421,7 @@ pub struct Resolver<'a, 'b: 'a> { block_map: NodeMap>, module_map: FxHashMap>, extern_module_map: FxHashMap<(DefId, bool /* MacrosOnly? */), Module<'a>>, + binding_parent_modules: FxHashMap>, Module<'a>>, pub make_glob_map: bool, /// Maps imports to the names of items actually imported (this actually maps @@ -1714,6 +1725,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { module_map, block_map: NodeMap(), extern_module_map: FxHashMap(), + binding_parent_modules: FxHashMap(), make_glob_map: make_glob_map == MakeGlobMap::Yes, glob_map: NodeMap(), @@ -4596,6 +4608,31 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> { vis.is_accessible_from(module.normal_ancestor_id, self) } + fn set_binding_parent_module(&mut self, binding: &'a NameBinding<'a>, module: Module<'a>) { + if let Some(old_module) = self.binding_parent_modules.insert(PtrKey(binding), module) { + if !ptr::eq(module, old_module) { + span_bug!(binding.span, "parent module is reset for binding"); + } + } + } + + fn disambiguate_legacy_vs_modern( + &self, + legacy: &'a NameBinding<'a>, + modern: &'a NameBinding<'a>, + ) -> bool { + // Some non-controversial subset of ambiguities "modern macro name" vs "macro_rules" + // is disambiguated to mitigate regressions from macro modularization. + // Scoping for `macro_rules` behaves like scoping for `let` at module level, in general. + match (self.binding_parent_modules.get(&PtrKey(legacy)), + self.binding_parent_modules.get(&PtrKey(modern))) { + (Some(legacy), Some(modern)) => + legacy.normal_ancestor_id == modern.normal_ancestor_id && + modern.is_ancestor_of(legacy), + _ => false, + } + } + fn report_ambiguity_error(&self, ident: Ident, b1: &NameBinding, b2: &NameBinding) { let participle = |is_import: bool| if is_import { "imported" } else { "defined" }; let msg1 = diff --git a/src/librustc_resolve/macros.rs b/src/librustc_resolve/macros.rs index 7b5e3ca078ba6..ff6e8a96f30ba 100644 --- a/src/librustc_resolve/macros.rs +++ b/src/librustc_resolve/macros.rs @@ -957,11 +957,13 @@ impl<'a, 'cl> Resolver<'a, 'cl> { }, (Some(legacy_binding), Ok((binding, FromPrelude(from_prelude)))) if legacy_binding.def() != binding.def_ignoring_ambiguity() && - (!from_prelude || + (!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 @@ -1097,6 +1099,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> { let def = Def::Macro(def_id, MacroKind::Bang); let vis = ty::Visibility::Invisible; // Doesn't matter for legacy bindings let binding = (def, vis, item.span, expansion).to_name_binding(self.arenas); + self.set_binding_parent_module(binding, self.current_module); let legacy_binding = self.arenas.alloc_legacy_binding(LegacyBinding { parent_legacy_scope: *current_legacy_scope, binding, ident }); diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index e689e6d70fdf4..4cf1a0d89356e 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -478,6 +478,7 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> { binding: &'a NameBinding<'a>) -> Result<(), &'a NameBinding<'a>> { self.check_reserved_macro_name(ident, ns); + self.set_binding_parent_module(binding, module); self.update_resolution(module, ident, ns, |this, resolution| { if let Some(old_binding) = resolution.binding { if binding.is_glob_import() { diff --git a/src/test/ui/imports/macros.rs b/src/test/ui/imports/macros.rs index 47ab8fc6c2f75..f7dc7ac9eac81 100644 --- a/src/test/ui/imports/macros.rs +++ b/src/test/ui/imports/macros.rs @@ -45,7 +45,7 @@ mod m3 { mod m4 { macro_rules! m { () => {} } use two_macros::m; - m!(); //~ ERROR ambiguous + m!(); } fn main() {} diff --git a/src/test/ui/imports/macros.stderr b/src/test/ui/imports/macros.stderr index c54101fc6e6a8..209d449dfd840 100644 --- a/src/test/ui/imports/macros.stderr +++ b/src/test/ui/imports/macros.stderr @@ -1,20 +1,3 @@ -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[E0659]: `m` is ambiguous --> $DIR/macros.rs:26:5 | @@ -51,6 +34,6 @@ LL | use two_macros::m; | ^^^^^^^^^^^^^ = note: macro-expanded macro imports do not shadow -error: aborting due to 3 previous errors +error: aborting due to 2 previous errors For more information about this error, try `rustc --explain E0659`. diff --git a/src/test/ui/macros/ambiguity-legacy-vs-modern.rs b/src/test/ui/macros/ambiguity-legacy-vs-modern.rs new file mode 100644 index 0000000000000..216b9dd0526d2 --- /dev/null +++ b/src/test/ui/macros/ambiguity-legacy-vs-modern.rs @@ -0,0 +1,46 @@ +// Some non-controversial subset of ambiguities "modern macro name" vs "macro_rules" +// is disambiguated to mitigate regressions from macro modularization. +// Scoping for `macro_rules` behaves like scoping for `let` at module level, in general. + +#![feature(decl_macro)] + +fn same_unnamed_mod() { + macro m() { 0 } + + macro_rules! m { () => (()) } + + m!() // OK +} + +fn nested_unnamed_mod() { + macro m() { 0 } + + { + macro_rules! m { () => (()) } + + m!() // OK + } +} + +fn nested_unnamed_mod_fail() { + macro_rules! m { () => (()) } + + { + macro m() { 0 } + + m!() //~ ERROR `m` is ambiguous + } +} + +fn nexted_named_mod_fail() { + macro m() { 0 } + + #[macro_use] + mod inner { + macro_rules! m { () => (()) } + } + + m!() //~ ERROR `m` is ambiguous +} + +fn main() {} diff --git a/src/test/ui/macros/ambiguity-legacy-vs-modern.stderr b/src/test/ui/macros/ambiguity-legacy-vs-modern.stderr new file mode 100644 index 0000000000000..b5e1e751737b4 --- /dev/null +++ b/src/test/ui/macros/ambiguity-legacy-vs-modern.stderr @@ -0,0 +1,37 @@ +error[E0659]: `m` is ambiguous + --> $DIR/ambiguity-legacy-vs-modern.rs:31:9 + | +LL | m!() //~ ERROR `m` is ambiguous + | ^ ambiguous name + | +note: `m` could refer to the name defined here + --> $DIR/ambiguity-legacy-vs-modern.rs:26:5 + | +LL | macro_rules! m { () => (()) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: `m` could also refer to the name defined here + --> $DIR/ambiguity-legacy-vs-modern.rs:29:9 + | +LL | macro m() { 0 } + | ^^^^^^^^^^^^^^^ + +error[E0659]: `m` is ambiguous + --> $DIR/ambiguity-legacy-vs-modern.rs:43:5 + | +LL | m!() //~ ERROR `m` is ambiguous + | ^ ambiguous name + | +note: `m` could refer to the name defined here + --> $DIR/ambiguity-legacy-vs-modern.rs:40:9 + | +LL | macro_rules! m { () => (()) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +note: `m` could also refer to the name defined here + --> $DIR/ambiguity-legacy-vs-modern.rs:36:5 + | +LL | macro m() { 0 } + | ^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0659`.