Skip to content

Commit 8431751

Browse files
petrochenkovpetrochenkov
committed
resolve: Turn the binding from #[macro_export] into a proper Import 
1 parent 637bfe6 commit 8431751

10 files changed

+96
-61
lines changed

‎compiler/rustc_resolve/src/build_reduced_graph.rs

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -56,21 +56,7 @@ impl<'a, Id: Into<DefId>> ToNameBinding<'a>
5656
impl<'a, Id: Into<DefId>> ToNameBinding<'a> for (Res, ty::Visibility<Id>, Span, LocalExpnId) {
5757
fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> &'a NameBinding<'a> {
5858
arenas.alloc_name_binding(NameBinding {
59-
kind: NameBindingKind::Res(self.0, false),
60-
ambiguity: None,
61-
vis: self.1.to_def_id(),
62-
span: self.2,
63-
expansion: self.3,
64-
})
65-
}
66-
}
67-
68-
struct IsMacroExport;
69-
70-
impl<'a> ToNameBinding<'a> for (Res, ty::Visibility, Span, LocalExpnId, IsMacroExport) {
71-
fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> &'a NameBinding<'a> {
72-
arenas.alloc_name_binding(NameBinding {
73-
kind: NameBindingKind::Res(self.0, true),
59+
kind: NameBindingKind::Res(self.0),
7460
ambiguity: None,
7561
vis: self.1.to_def_id(),
7662
span: self.2,
@@ -1267,8 +1253,22 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
12671253
let binding = (res, vis, span, expansion).to_name_binding(self.r.arenas);
12681254
self.r.set_binding_parent_module(binding, parent_scope.module);
12691255
if is_macro_export {
1270-
let module = self.r.graph_root;
1271-
self.r.define(module, ident, MacroNS, (res, vis, span, expansion, IsMacroExport));
1256+
let import = self.r.arenas.alloc_import(Import {
1257+
kind: ImportKind::MacroExport,
1258+
root_id: item.id,
1259+
parent_scope: self.parent_scope,
1260+
imported_module: Cell::new(None),
1261+
has_attributes: false,
1262+
use_span_with_attributes: span,
1263+
use_span: span,
1264+
root_span: span,
1265+
span: span,
1266+
module_path: Vec::new(),
1267+
vis: Cell::new(Some(vis)),
1268+
used: Cell::new(true),
1269+
});
1270+
let import_binding = self.r.import(binding, import);
1271+
self.r.define(self.r.graph_root, ident, MacroNS, import_binding);
12721272
} else {
12731273
self.r.check_reserved_macro_name(ident, res);
12741274
self.insert_unused_macro(ident, def_id, item.id, &rule_spans);

‎compiler/rustc_resolve/src/diagnostics.rs

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -190,12 +190,12 @@ impl<'a> Resolver<'a> {
190190
ModuleKind::Block => "block",
191191
};
192192

193-
let old_noun = match old_binding.is_import() {
193+
let old_noun = match old_binding.is_import_user_facing() {
194194
true => "import",
195195
false => "definition",
196196
};
197197

198-
let new_participle = match new_binding.is_import() {
198+
let new_participle = match new_binding.is_import_user_facing() {
199199
true => "imported",
200200
false => "defined",
201201
};
@@ -226,7 +226,7 @@ impl<'a> Resolver<'a> {
226226
true => struct_span_err!(self.session, span, E0254, "{}", msg),
227227
false => struct_span_err!(self.session, span, E0260, "{}", msg),
228228
},
229-
_ => match (old_binding.is_import(), new_binding.is_import()) {
229+
_ => match (old_binding.is_import_user_facing(), new_binding.is_import_user_facing()) {
230230
(false, false) => struct_span_err!(self.session, span, E0428, "{}", msg),
231231
(true, true) => struct_span_err!(self.session, span, E0252, "{}", msg),
232232
_ => struct_span_err!(self.session, span, E0255, "{}", msg),
@@ -248,14 +248,18 @@ impl<'a> Resolver<'a> {
248248

249249
// See https://github.com/rust-lang/rust/issues/32354
250250
use NameBindingKind::Import;
251+
let can_suggest = |binding: &NameBinding<'_>, import: &self::Import<'_>| {
252+
!binding.span.is_dummy()
253+
&& !matches!(import.kind, ImportKind::MacroUse | ImportKind::MacroExport)
254+
};
251255
let import = match (&new_binding.kind, &old_binding.kind) {
252256
// If there are two imports where one or both have attributes then prefer removing the
253257
// import without attributes.
254258
(Import { import: new, .. }, Import { import: old, .. })
255259
if {
256-
!new_binding.span.is_dummy()
257-
&& !old_binding.span.is_dummy()
258-
&& (new.has_attributes || old.has_attributes)
260+
(new.has_attributes || old.has_attributes)
261+
&& can_suggest(old_binding, old)
262+
&& can_suggest(new_binding, new)
259263
} =>
260264
{
261265
if old.has_attributes {
@@ -265,10 +269,10 @@ impl<'a> Resolver<'a> {
265269
}
266270
}
267271
// Otherwise prioritize the new binding.
268-
(Import { import, .. }, other) if !new_binding.span.is_dummy() => {
272+
(Import { import, .. }, other) if can_suggest(new_binding, import) => {
269273
Some((import, new_binding.span, other.is_import()))
270274
}
271-
(other, Import { import, .. }) if !old_binding.span.is_dummy() => {
275+
(other, Import { import, .. }) if can_suggest(old_binding, import) => {
272276
Some((import, old_binding.span, other.is_import()))
273277
}
274278
_ => None,
@@ -1683,7 +1687,7 @@ impl<'a> Resolver<'a> {
16831687
let a = if built_in.is_empty() { res.article() } else { "a" };
16841688
format!("{a}{built_in} {thing}{from}", thing = res.descr())
16851689
} else {
1686-
let introduced = if b.is_import() { "imported" } else { "defined" };
1690+
let introduced = if b.is_import_user_facing() { "imported" } else { "defined" };
16871691
format!("the {thing} {introduced} here", thing = res.descr())
16881692
}
16891693
}
@@ -1742,10 +1746,10 @@ impl<'a> Resolver<'a> {
17421746
/// If the binding refers to a tuple struct constructor with fields,
17431747
/// returns the span of its fields.
17441748
fn ctor_fields_span(&self, binding: &NameBinding<'_>) -> Option<Span> {
1745-
if let NameBindingKind::Res(
1746-
Res::Def(DefKind::Ctor(CtorOf::Struct, CtorKind::Fn), ctor_def_id),
1747-
_,
1748-
) = binding.kind
1749+
if let NameBindingKind::Res(Res::Def(
1750+
DefKind::Ctor(CtorOf::Struct, CtorKind::Fn),
1751+
ctor_def_id,
1752+
)) = binding.kind
17491753
{
17501754
let def_id = self.parent(ctor_def_id);
17511755
let fields = self.field_names.get(&def_id)?;
@@ -1789,7 +1793,9 @@ impl<'a> Resolver<'a> {
17891793
next_ident = source;
17901794
Some(binding)
17911795
}
1792-
ImportKind::Glob { .. } | ImportKind::MacroUse => Some(binding),
1796+
ImportKind::Glob { .. } | ImportKind::MacroUse | ImportKind::MacroExport => {
1797+
Some(binding)
1798+
}
17931799
ImportKind::ExternCrate { .. } => None,
17941800
},
17951801
_ => None,

‎compiler/rustc_resolve/src/effective_visibilities.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,11 @@ impl<'r, 'a> EffectiveVisibilitiesVisitor<'r, 'a> {
8888
// here, but `macro_use` imports always refer to external items,
8989
// so it doesn't matter and we can just do nothing.
9090
}
91+
ImportKind::MacroExport => {
92+
// In theory we should reset the parent id to something public
93+
// here, but it has the same effect as leaving the previous parent,
94+
// so we can just do nothing.
95+
}
9196
}
9297

9398
level = Level::Reexported;
@@ -152,13 +157,6 @@ impl<'r, 'ast> Visitor<'ast> for EffectiveVisibilitiesVisitor<'ast, 'r> {
152157
self.update(def_id, Visibility::Public, parent_id, Level::Direct);
153158
}
154159

155-
// Only exported `macro_rules!` items are public, but they always are
156-
ast::ItemKind::MacroDef(ref macro_def) if macro_def.macro_rules => {
157-
let parent_id = self.r.local_parent(def_id);
158-
let vis = self.r.visibilities[&def_id];
159-
self.update(def_id, vis, parent_id, Level::Direct);
160-
}
161-
162160
ast::ItemKind::Mod(..) => {
163161
self.set_bindings_effective_visibilities(def_id);
164162
visit::walk_item(self, item);

‎compiler/rustc_resolve/src/ident.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use crate::late::{
1919
};
2020
use crate::macros::{sub_namespace_match, MacroRulesScope};
2121
use crate::{AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, Determinacy, Finalize};
22-
use crate::{ImportKind, LexicalScopeBinding, Module, ModuleKind, ModuleOrUniformRoot};
22+
use crate::{Import, ImportKind, LexicalScopeBinding, Module, ModuleKind, ModuleOrUniformRoot};
2323
use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, PrivacyError, Res};
2424
use crate::{ResolutionError, Resolver, Scope, ScopeSet, Segment, ToNameBinding, Weak};
2525

@@ -915,7 +915,11 @@ impl<'a> Resolver<'a> {
915915
}
916916

917917
if !restricted_shadowing && binding.expansion != LocalExpnId::ROOT {
918-
if let NameBindingKind::Res(_, true) = binding.kind {
918+
if let NameBindingKind::Import {
919+
import: Import { kind: ImportKind::MacroExport, .. },
920+
..
921+
} = binding.kind
922+
{
919923
self.macro_expanded_macro_export_errors.insert((path_span, binding.span));
920924
}
921925
}

‎compiler/rustc_resolve/src/imports.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ pub enum ImportKind<'a> {
7373
id: NodeId,
7474
},
7575
MacroUse,
76+
MacroExport,
7677
}
7778

7879
/// Manually implement `Debug` for `ImportKind` because the `source/target_bindings`
@@ -113,6 +114,7 @@ impl<'a> std::fmt::Debug for ImportKind<'a> {
113114
.field("id", id)
114115
.finish(),
115116
MacroUse => f.debug_struct("MacroUse").finish(),
117+
MacroExport => f.debug_struct("MacroExport").finish(),
116118
}
117119
}
118120
}
@@ -177,7 +179,7 @@ impl<'a> Import<'a> {
177179
ImportKind::Single { id, .. }
178180
| ImportKind::Glob { id, .. }
179181
| ImportKind::ExternCrate { id, .. } => Some(id),
180-
ImportKind::MacroUse => None,
182+
ImportKind::MacroUse | ImportKind::MacroExport => None,
181183
}
182184
}
183185
}
@@ -883,7 +885,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
883885
match binding.kind {
884886
// Never suggest the name that has binding error
885887
// i.e., the name that cannot be previously resolved
886-
NameBindingKind::Res(Res::Err, _) => None,
888+
NameBindingKind::Res(Res::Err) => None,
887889
_ => Some(i.name),
888890
}
889891
}
@@ -1014,7 +1016,7 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
10141016
let mut err =
10151017
struct_span_err!(self.r.session, import.span, E0364, "{error_msg}");
10161018
match binding.kind {
1017-
NameBindingKind::Res(Res::Def(DefKind::Macro(_), def_id), _)
1019+
NameBindingKind::Res(Res::Def(DefKind::Macro(_), def_id))
10181020
// exclude decl_macro
10191021
if self.r.get_macro_by_def_id(def_id).macro_rules =>
10201022
{
@@ -1235,5 +1237,6 @@ fn import_kind_to_string(import_kind: &ImportKind<'_>) -> String {
12351237
ImportKind::Glob { .. } => "*".to_string(),
12361238
ImportKind::ExternCrate { .. } => "<extern crate>".to_string(),
12371239
ImportKind::MacroUse => "#[macro_use]".to_string(),
1240+
ImportKind::MacroExport => "#[macro_export]".to_string(),
12381241
}
12391242
}

‎compiler/rustc_resolve/src/lib.rs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ impl<'a> ToNameBinding<'a> for &'a NameBinding<'a> {
646646

647647
#[derive(Clone, Debug)]
648648
enum NameBindingKind<'a> {
649-
Res(Res, /* is_macro_export */ bool),
649+
Res(Res),
650650
Module(Module<'a>),
651651
Import { binding: &'a NameBinding<'a>, import: &'a Import<'a>, used: Cell<bool> },
652652
}
@@ -745,7 +745,7 @@ impl<'a> NameBinding<'a> {
745745

746746
fn res(&self) -> Res {
747747
match self.kind {
748-
NameBindingKind::Res(res, _) => res,
748+
NameBindingKind::Res(res) => res,
749749
NameBindingKind::Module(module) => module.res().unwrap(),
750750
NameBindingKind::Import { binding, .. } => binding.res(),
751751
}
@@ -762,10 +762,10 @@ impl<'a> NameBinding<'a> {
762762
fn is_possibly_imported_variant(&self) -> bool {
763763
match self.kind {
764764
NameBindingKind::Import { binding, .. } => binding.is_possibly_imported_variant(),
765-
NameBindingKind::Res(
766-
Res::Def(DefKind::Variant | DefKind::Ctor(CtorOf::Variant, ..), _),
765+
NameBindingKind::Res(Res::Def(
766+
DefKind::Variant | DefKind::Ctor(CtorOf::Variant, ..),
767767
_,
768-
) => true,
768+
)) => true,
769769
NameBindingKind::Res(..) | NameBindingKind::Module(..) => false,
770770
}
771771
}
@@ -788,6 +788,13 @@ impl<'a> NameBinding<'a> {
788788
matches!(self.kind, NameBindingKind::Import { .. })
789789
}
790790

791+
/// The binding introduced by `#[macro_export] macro_rules` is a public import, but it might
792+
/// not be perceived as such by users, so treat it as a non-import in some diagnostics.
793+
fn is_import_user_facing(&self) -> bool {
794+
matches!(self.kind, NameBindingKind::Import { import, .. }
795+
if !matches!(import.kind, ImportKind::MacroExport))
796+
}
797+
791798
fn is_glob_import(&self) -> bool {
792799
match self.kind {
793800
NameBindingKind::Import { import, .. } => import.is_glob(),
@@ -1283,7 +1290,7 @@ impl<'a> Resolver<'a> {
12831290

12841291
arenas,
12851292
dummy_binding: arenas.alloc_name_binding(NameBinding {
1286-
kind: NameBindingKind::Res(Res::Err, false),
1293+
kind: NameBindingKind::Res(Res::Err),
12871294
ambiguity: None,
12881295
expansion: LocalExpnId::ROOT,
12891296
span: DUMMY_SP,
@@ -1998,11 +2005,7 @@ impl<'a> Resolver<'a> {
19982005

19992006
// Items that go to reexport table encoded to metadata and visible through it to other crates.
20002007
fn is_reexport(&self, binding: &NameBinding<'a>) -> Option<def::Res<!>> {
2001-
// FIXME: Consider changing the binding inserted by `#[macro_export] macro_rules`
2002-
// into the crate root to actual `NameBindingKind::Import`.
2003-
if binding.is_import()
2004-
|| matches!(binding.kind, NameBindingKind::Res(_, _is_macro_export @ true))
2005-
{
2008+
if binding.is_import() {
20062009
let res = binding.res().expect_non_local();
20072010
// Ambiguous imports are treated as errors at this point and are
20082011
// not exposed to other crates (see #36837 for more details).

‎src/test/ui/macros/issue-38715.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,17 @@
11
#[macro_export]
2-
macro_rules! foo { ($i:ident) => {} }
2+
macro_rules! foo { () => {} }
33

44
#[macro_export]
55
macro_rules! foo { () => {} } //~ ERROR the name `foo` is defined multiple times
66

7+
mod inner1 {
8+
#[macro_export]
9+
macro_rules! bar { () => {} }
10+
}
11+
12+
mod inner2 {
13+
#[macro_export]
14+
macro_rules! bar { () => {} } //~ ERROR the name `bar` is defined multiple times
15+
}
16+
717
fn main() {}

‎src/test/ui/macros/issue-38715.stderr

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,25 @@
11
error[E0428]: the name `foo` is defined multiple times
22
--> $DIR/issue-38715.rs:5:1
33
|
4-
LL | macro_rules! foo { ($i:ident) => {} }
4+
LL | macro_rules! foo { () => {} }
55
| ---------------- previous definition of the macro `foo` here
66
...
77
LL | macro_rules! foo { () => {} }
88
| ^^^^^^^^^^^^^^^^ `foo` redefined here
99
|
1010
= note: `foo` must be defined only once in the macro namespace of this module
1111

12-
error: aborting due to previous error
12+
error[E0428]: the name `bar` is defined multiple times
13+
--> $DIR/issue-38715.rs:14:5
14+
|
15+
LL | macro_rules! bar { () => {} }
16+
| ---------------- previous definition of the macro `bar` here
17+
...
18+
LL | macro_rules! bar { () => {} }
19+
| ^^^^^^^^^^^^^^^^ `bar` redefined here
20+
|
21+
= note: `bar` must be defined only once in the macro namespace of this module
22+
23+
error: aborting due to 2 previous errors
1324

1425
For more information about this error, try `rustc --explain E0428`.

‎src/test/ui/privacy/effective_visibilities.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,13 @@ mod outer { //~ ERROR Direct: pub(crate), Reexported: pub(crate), Reachable: pub
3838
}
3939

4040
#[rustc_effective_visibility]
41-
macro_rules! none_macro { //~ Direct: pub(crate), Reexported: pub(crate), Reachable: pub(crate), ReachableThroughImplTrait: pub(crate)
41+
macro_rules! none_macro { //~ ERROR not in the table
4242
() => {};
4343
}
4444

4545
#[macro_export]
4646
#[rustc_effective_visibility]
47-
macro_rules! public_macro { //~ Direct: pub, Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
47+
macro_rules! public_macro { //~ ERROR Direct: pub(self), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
4848
() => {};
4949
}
5050

‎src/test/ui/privacy/effective_visibilities.stderr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,13 @@ error: Direct: pub(crate), Reexported: pub, Reachable: pub, ReachableThroughImpl
6464
LL | PubUnion,
6565
| ^^^^^^^^
6666

67-
error: Direct: pub(crate), Reexported: pub(crate), Reachable: pub(crate), ReachableThroughImplTrait: pub(crate)
67+
error: not in the table
6868
--> $DIR/effective_visibilities.rs:41:5
6969
|
7070
LL | macro_rules! none_macro {
7171
| ^^^^^^^^^^^^^^^^^^^^^^^
7272

73-
error: Direct: pub, Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
73+
error: Direct: pub(self), Reexported: pub, Reachable: pub, ReachableThroughImplTrait: pub
7474
--> $DIR/effective_visibilities.rs:47:5
7575
|
7676
LL | macro_rules! public_macro {

0 commit comments

Comments
 (0)