Skip to content

Commit 96eee8a

Browse files
petrochenkovpietroalbini
authored andcommitted
resolve: Prefer macro_rules definitions to in-module macro definitions in some cases
1 parent f11dae2 commit 96eee8a

File tree

7 files changed

+130
-23
lines changed

7 files changed

+130
-23
lines changed

src/librustc_resolve/lib.rs

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,10 @@ use syntax_pos::{Span, DUMMY_SP, MultiSpan};
7171
use errors::{Applicability, DiagnosticBuilder, DiagnosticId};
7272

7373
use std::cell::{Cell, RefCell};
74-
use std::cmp;
74+
use std::{cmp, fmt, iter, ptr};
7575
use std::collections::BTreeSet;
76-
use std::fmt;
77-
use std::iter;
7876
use std::mem::replace;
77+
use rustc_data_structures::ptr_key::PtrKey;
7978
use rustc_data_structures::sync::Lrc;
8079

8180
use resolve_imports::{ImportDirective, ImportDirectiveSubclass, NameResolution, ImportResolver};
@@ -1113,6 +1112,17 @@ impl<'a> ModuleData<'a> {
11131112
fn nearest_item_scope(&'a self) -> Module<'a> {
11141113
if self.is_trait() { self.parent.unwrap() } else { self }
11151114
}
1115+
1116+
fn is_ancestor_of(&self, mut other: &Self) -> bool {
1117+
while !ptr::eq(self, other) {
1118+
if let Some(parent) = other.parent {
1119+
other = parent;
1120+
} else {
1121+
return false;
1122+
}
1123+
}
1124+
true
1125+
}
11161126
}
11171127

11181128
impl<'a> fmt::Debug for ModuleData<'a> {
@@ -1407,6 +1417,7 @@ pub struct Resolver<'a, 'b: 'a> {
14071417
block_map: NodeMap<Module<'a>>,
14081418
module_map: FxHashMap<DefId, Module<'a>>,
14091419
extern_module_map: FxHashMap<(DefId, bool /* MacrosOnly? */), Module<'a>>,
1420+
binding_parent_modules: FxHashMap<PtrKey<'a, NameBinding<'a>>, Module<'a>>,
14101421

14111422
pub make_glob_map: bool,
14121423
/// Maps imports to the names of items actually imported (this actually maps
@@ -1709,6 +1720,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
17091720
module_map,
17101721
block_map: NodeMap(),
17111722
extern_module_map: FxHashMap(),
1723+
binding_parent_modules: FxHashMap(),
17121724

17131725
make_glob_map: make_glob_map == MakeGlobMap::Yes,
17141726
glob_map: NodeMap(),
@@ -4526,6 +4538,31 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
45264538
vis.is_accessible_from(module.normal_ancestor_id, self)
45274539
}
45284540

4541+
fn set_binding_parent_module(&mut self, binding: &'a NameBinding<'a>, module: Module<'a>) {
4542+
if let Some(old_module) = self.binding_parent_modules.insert(PtrKey(binding), module) {
4543+
if !ptr::eq(module, old_module) {
4544+
span_bug!(binding.span, "parent module is reset for binding");
4545+
}
4546+
}
4547+
}
4548+
4549+
fn disambiguate_legacy_vs_modern(
4550+
&self,
4551+
legacy: &'a NameBinding<'a>,
4552+
modern: &'a NameBinding<'a>,
4553+
) -> bool {
4554+
// Some non-controversial subset of ambiguities "modern macro name" vs "macro_rules"
4555+
// is disambiguated to mitigate regressions from macro modularization.
4556+
// Scoping for `macro_rules` behaves like scoping for `let` at module level, in general.
4557+
match (self.binding_parent_modules.get(&PtrKey(legacy)),
4558+
self.binding_parent_modules.get(&PtrKey(modern))) {
4559+
(Some(legacy), Some(modern)) =>
4560+
legacy.normal_ancestor_id == modern.normal_ancestor_id &&
4561+
modern.is_ancestor_of(legacy),
4562+
_ => false,
4563+
}
4564+
}
4565+
45294566
fn report_ambiguity_error(&self, ident: Ident, b1: &NameBinding, b2: &NameBinding) {
45304567
let participle = |is_import: bool| if is_import { "imported" } else { "defined" };
45314568
let msg1 =

src/librustc_resolve/macros.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -956,11 +956,13 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
956956
},
957957
(Some(legacy_binding), Ok((binding, FromPrelude(from_prelude))))
958958
if legacy_binding.def() != binding.def_ignoring_ambiguity() &&
959-
(!from_prelude ||
959+
(!from_prelude &&
960+
!self.disambiguate_legacy_vs_modern(legacy_binding, binding) ||
960961
legacy_binding.may_appear_after(parent_scope.expansion, binding)) => {
961962
self.report_ambiguity_error(ident, legacy_binding, binding);
962963
},
963964
// OK, non-macro-expanded legacy wins over prelude even if defs are different
965+
// Also, non-macro-expanded legacy wins over modern from the same module
964966
// Also, legacy and modern can co-exist if their defs are same
965967
(Some(legacy_binding), Ok(_)) |
966968
// OK, unambiguous resolution
@@ -1096,6 +1098,7 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
10961098
let def = Def::Macro(def_id, MacroKind::Bang);
10971099
let vis = ty::Visibility::Invisible; // Doesn't matter for legacy bindings
10981100
let binding = (def, vis, item.span, expansion).to_name_binding(self.arenas);
1101+
self.set_binding_parent_module(binding, self.current_module);
10991102
let legacy_binding = self.arenas.alloc_legacy_binding(LegacyBinding {
11001103
parent_legacy_scope: *current_legacy_scope, binding, ident
11011104
});

src/librustc_resolve/resolve_imports.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,7 @@ impl<'a, 'crateloader> Resolver<'a, 'crateloader> {
478478
binding: &'a NameBinding<'a>)
479479
-> Result<(), &'a NameBinding<'a>> {
480480
self.check_reserved_macro_name(ident, ns);
481+
self.set_binding_parent_module(binding, module);
481482
self.update_resolution(module, ident, ns, |this, resolution| {
482483
if let Some(old_binding) = resolution.binding {
483484
if binding.is_glob_import() {

src/test/ui/imports/macros.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ mod m3 {
4545
mod m4 {
4646
macro_rules! m { () => {} }
4747
use two_macros::m;
48-
m!(); //~ ERROR ambiguous
48+
m!();
4949
}
5050

5151
fn main() {}

src/test/ui/imports/macros.stderr

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,3 @@
1-
error[E0659]: `m` is ambiguous
2-
--> $DIR/macros.rs:48:5
3-
|
4-
LL | m!(); //~ ERROR ambiguous
5-
| ^ ambiguous name
6-
|
7-
note: `m` could refer to the name defined here
8-
--> $DIR/macros.rs:46:5
9-
|
10-
LL | macro_rules! m { () => {} }
11-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
12-
note: `m` could also refer to the name imported here
13-
--> $DIR/macros.rs:47:9
14-
|
15-
LL | use two_macros::m;
16-
| ^^^^^^^^^^^^^
17-
181
error[E0659]: `m` is ambiguous
192
--> $DIR/macros.rs:26:5
203
|
@@ -51,6 +34,6 @@ LL | use two_macros::m;
5134
| ^^^^^^^^^^^^^
5235
= note: macro-expanded macro imports do not shadow
5336

54-
error: aborting due to 3 previous errors
37+
error: aborting due to 2 previous errors
5538

5639
For more information about this error, try `rustc --explain E0659`.
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// Some non-controversial subset of ambiguities "modern macro name" vs "macro_rules"
2+
// is disambiguated to mitigate regressions from macro modularization.
3+
// Scoping for `macro_rules` behaves like scoping for `let` at module level, in general.
4+
5+
#![feature(decl_macro)]
6+
7+
fn same_unnamed_mod() {
8+
macro m() { 0 }
9+
10+
macro_rules! m { () => (()) }
11+
12+
m!() // OK
13+
}
14+
15+
fn nested_unnamed_mod() {
16+
macro m() { 0 }
17+
18+
{
19+
macro_rules! m { () => (()) }
20+
21+
m!() // OK
22+
}
23+
}
24+
25+
fn nested_unnamed_mod_fail() {
26+
macro_rules! m { () => (()) }
27+
28+
{
29+
macro m() { 0 }
30+
31+
m!() //~ ERROR `m` is ambiguous
32+
}
33+
}
34+
35+
fn nexted_named_mod_fail() {
36+
macro m() { 0 }
37+
38+
#[macro_use]
39+
mod inner {
40+
macro_rules! m { () => (()) }
41+
}
42+
43+
m!() //~ ERROR `m` is ambiguous
44+
}
45+
46+
fn main() {}
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
error[E0659]: `m` is ambiguous
2+
--> $DIR/ambiguity-legacy-vs-modern.rs:31:9
3+
|
4+
LL | m!() //~ ERROR `m` is ambiguous
5+
| ^ ambiguous name
6+
|
7+
note: `m` could refer to the name defined here
8+
--> $DIR/ambiguity-legacy-vs-modern.rs:26:5
9+
|
10+
LL | macro_rules! m { () => (()) }
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12+
note: `m` could also refer to the name defined here
13+
--> $DIR/ambiguity-legacy-vs-modern.rs:29:9
14+
|
15+
LL | macro m() { 0 }
16+
| ^^^^^^^^^^^^^^^
17+
18+
error[E0659]: `m` is ambiguous
19+
--> $DIR/ambiguity-legacy-vs-modern.rs:43:5
20+
|
21+
LL | m!() //~ ERROR `m` is ambiguous
22+
| ^ ambiguous name
23+
|
24+
note: `m` could refer to the name defined here
25+
--> $DIR/ambiguity-legacy-vs-modern.rs:40:9
26+
|
27+
LL | macro_rules! m { () => (()) }
28+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
29+
note: `m` could also refer to the name defined here
30+
--> $DIR/ambiguity-legacy-vs-modern.rs:36:5
31+
|
32+
LL | macro m() { 0 }
33+
| ^^^^^^^^^^^^^^^
34+
35+
error: aborting due to 2 previous errors
36+
37+
For more information about this error, try `rustc --explain E0659`.

0 commit comments

Comments
 (0)