Skip to content

Commit

Permalink
Rollup merge of rust-lang#103760 - petrochenkov:macimp, r=cjgillot
Browse files Browse the repository at this point in the history 
resolve: Turn the binding from `#[macro_export]` into a proper `Import`

Continuation of rust-lang#91795.

```rust
#[macro_export]
macro_rules! m { /*...*/ }
```
is desugared to something like
```rust
macro_rules! m { /*...*/ } // Non-modularized macro_rules item

pub use m; // It's modularized reexport
```

This PR adjusts the internal representation to better match this model.
  • Loading branch information
@Dylan-DPC
Dylan-DPC authored Nov 1, 2022
2 parents 9f603fe + 8431751 commit 68afa32
Show file tree
Hide file tree
Showing 11 changed files with 237 additions and 164 deletions.
55 changes: 22 additions & 33 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,21 +56,7 @@ impl<'a, Id: Into<DefId>> ToNameBinding<'a>
impl<'a, Id: Into<DefId>> ToNameBinding<'a> for (Res, ty::Visibility<Id>, Span, LocalExpnId) {
fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> &'a NameBinding<'a> {
arenas.alloc_name_binding(NameBinding {
kind: NameBindingKind::Res(self.0, false),
ambiguity: None,
vis: self.1.to_def_id(),
span: self.2,
expansion: self.3,
})
}
}

struct IsMacroExport;

impl<'a> ToNameBinding<'a> for (Res, ty::Visibility, Span, LocalExpnId, IsMacroExport) {
fn to_name_binding(self, arenas: &'a ResolverArenas<'a>) -> &'a NameBinding<'a> {
arenas.alloc_name_binding(NameBinding {
kind: NameBindingKind::Res(self.0, true),
kind: NameBindingKind::Res(self.0),
ambiguity: None,
vis: self.1.to_def_id(),
span: self.2,
Expand Down Expand Up @@ -364,7 +350,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
module_path: Vec<Segment>,
kind: ImportKind<'a>,
span: Span,
id: NodeId,
item: &ast::Item,
root_span: Span,
root_id: NodeId,
Expand All @@ -377,7 +362,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
module_path,
imported_module: Cell::new(None),
span,
id,
use_span: item.span,
use_span_with_attributes: item.span_with_attributes(),
has_attributes: !item.attrs.is_empty(),
Expand Down Expand Up @@ -574,27 +558,20 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
},
type_ns_only,
nested,
id,
additional_ids: (id1, id2),
};

self.add_import(
module_path,
kind,
use_tree.span,
id,
item,
root_span,
item.id,
vis,
);
self.add_import(module_path, kind, use_tree.span, item, root_span, item.id, vis);
}
ast::UseTreeKind::Glob => {
let kind = ImportKind::Glob {
is_prelude: self.r.session.contains_name(&item.attrs, sym::prelude_import),
max_vis: Cell::new(None),
id,
};
self.r.visibilities.insert(self.r.local_def_id(id), vis);
self.add_import(prefix, kind, use_tree.span, id, item, root_span, item.id, vis);
self.add_import(prefix, kind, use_tree.span, item, root_span, item.id, vis);
}
ast::UseTreeKind::Nested(ref items) => {
// Ensure there is at most one `self` in the list
Expand Down Expand Up @@ -881,9 +858,8 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
})
.unwrap_or((true, None, self.r.dummy_binding));
let import = self.r.arenas.alloc_import(Import {
kind: ImportKind::ExternCrate { source: orig_name, target: ident },
kind: ImportKind::ExternCrate { source: orig_name, target: ident, id: item.id },
root_id: item.id,
id: item.id,
parent_scope: self.parent_scope,
imported_module: Cell::new(module),
has_attributes: !item.attrs.is_empty(),
Expand Down Expand Up @@ -1118,7 +1094,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
this.r.arenas.alloc_import(Import {
kind: ImportKind::MacroUse,
root_id: item.id,
id: item.id,
parent_scope: this.parent_scope,
imported_module: Cell::new(Some(ModuleOrUniformRoot::Module(module))),
use_span_with_attributes: item.span_with_attributes(),
Expand Down Expand Up @@ -1278,8 +1253,22 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
let binding = (res, vis, span, expansion).to_name_binding(self.r.arenas);
self.r.set_binding_parent_module(binding, parent_scope.module);
if is_macro_export {
let module = self.r.graph_root;
self.r.define(module, ident, MacroNS, (res, vis, span, expansion, IsMacroExport));
let import = self.r.arenas.alloc_import(Import {
kind: ImportKind::MacroExport,
root_id: item.id,
parent_scope: self.parent_scope,
imported_module: Cell::new(None),
has_attributes: false,
use_span_with_attributes: span,
use_span: span,
root_span: span,
span: span,
module_path: Vec::new(),
vis: Cell::new(Some(vis)),
used: Cell::new(true),
});
let import_binding = self.r.import(binding, import);
self.r.define(self.r.graph_root, ident, MacroNS, import_binding);
} else {
self.r.check_reserved_macro_name(ident, res);
self.insert_unused_macro(ident, def_id, item.id, &rule_spans);
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_resolve/src/check_unused.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ impl Resolver<'_> {
if !import.span.is_dummy() {
self.lint_buffer.buffer_lint(
MACRO_USE_EXTERN_CRATE,
import.id,
import.root_id,
import.span,
"deprecated `#[macro_use]` attribute used to \
import macros should be replaced at use sites \
Expand All @@ -244,13 +244,13 @@ impl Resolver<'_> {
}
}
}
ImportKind::ExternCrate { .. } => {
let def_id = self.local_def_id(import.id);
ImportKind::ExternCrate { id, .. } => {
let def_id = self.local_def_id(id);
self.maybe_unused_extern_crates.push((def_id, import.span));
}
ImportKind::MacroUse => {
let msg = "unused `#[macro_use]` import";
self.lint_buffer.buffer_lint(UNUSED_IMPORTS, import.id, import.span, msg);
self.lint_buffer.buffer_lint(UNUSED_IMPORTS, import.root_id, import.span, msg);
}
_ => {}
}
Expand Down
36 changes: 21 additions & 15 deletions compiler/rustc_resolve/src/diagnostics.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,12 @@ impl<'a> Resolver<'a> {
ModuleKind::Block => "block",
};

let old_noun = match old_binding.is_import() {
let old_noun = match old_binding.is_import_user_facing() {
true => "import",
false => "definition",
};

let new_participle = match new_binding.is_import() {
let new_participle = match new_binding.is_import_user_facing() {
true => "imported",
false => "defined",
};
Expand Down Expand Up @@ -226,7 +226,7 @@ impl<'a> Resolver<'a> {
true => struct_span_err!(self.session, span, E0254, "{}", msg),
false => struct_span_err!(self.session, span, E0260, "{}", msg),
},
_ => match (old_binding.is_import(), new_binding.is_import()) {
_ => match (old_binding.is_import_user_facing(), new_binding.is_import_user_facing()) {
(false, false) => struct_span_err!(self.session, span, E0428, "{}", msg),
(true, true) => struct_span_err!(self.session, span, E0252, "{}", msg),
_ => struct_span_err!(self.session, span, E0255, "{}", msg),
Expand All @@ -248,14 +248,18 @@ impl<'a> Resolver<'a> {

// See https://github.com/rust-lang/rust/issues/32354
use NameBindingKind::Import;
let can_suggest = |binding: &NameBinding<'_>, import: &self::Import<'_>| {
!binding.span.is_dummy()
&& !matches!(import.kind, ImportKind::MacroUse | ImportKind::MacroExport)
};
let import = match (&new_binding.kind, &old_binding.kind) {
// If there are two imports where one or both have attributes then prefer removing the
// import without attributes.
(Import { import: new, .. }, Import { import: old, .. })
if {
!new_binding.span.is_dummy()
&& !old_binding.span.is_dummy()
&& (new.has_attributes || old.has_attributes)
(new.has_attributes || old.has_attributes)
&& can_suggest(old_binding, old)
&& can_suggest(new_binding, new)
} =>
{
if old.has_attributes {
Expand All @@ -265,10 +269,10 @@ impl<'a> Resolver<'a> {
}
}
// Otherwise prioritize the new binding.
(Import { import, .. }, other) if !new_binding.span.is_dummy() => {
(Import { import, .. }, other) if can_suggest(new_binding, import) => {
Some((import, new_binding.span, other.is_import()))
}
(other, Import { import, .. }) if !old_binding.span.is_dummy() => {
(other, Import { import, .. }) if can_suggest(old_binding, import) => {
Some((import, old_binding.span, other.is_import()))
}
_ => None,
Expand Down Expand Up @@ -353,7 +357,7 @@ impl<'a> Resolver<'a> {
}
}
}
ImportKind::ExternCrate { source, target } => {
ImportKind::ExternCrate { source, target, .. } => {
suggestion = Some(format!(
"extern crate {} as {};",
source.unwrap_or(target.name),
Expand Down Expand Up @@ -1683,7 +1687,7 @@ impl<'a> Resolver<'a> {
let a = if built_in.is_empty() { res.article() } else { "a" };
format!("{a}{built_in} {thing}{from}", thing = res.descr())
} else {
let introduced = if b.is_import() { "imported" } else { "defined" };
let introduced = if b.is_import_user_facing() { "imported" } else { "defined" };
format!("the {thing} {introduced} here", thing = res.descr())
}
}
Expand Down Expand Up @@ -1742,10 +1746,10 @@ impl<'a> Resolver<'a> {
/// If the binding refers to a tuple struct constructor with fields,
/// returns the span of its fields.
fn ctor_fields_span(&self, binding: &NameBinding<'_>) -> Option<Span> {
if let NameBindingKind::Res(
Res::Def(DefKind::Ctor(CtorOf::Struct, CtorKind::Fn), ctor_def_id),
_,
) = binding.kind
if let NameBindingKind::Res(Res::Def(
DefKind::Ctor(CtorOf::Struct, CtorKind::Fn),
ctor_def_id,
)) = binding.kind
{
let def_id = self.parent(ctor_def_id);
let fields = self.field_names.get(&def_id)?;
Expand Down Expand Up @@ -1789,7 +1793,9 @@ impl<'a> Resolver<'a> {
next_ident = source;
Some(binding)
}
ImportKind::Glob { .. } | ImportKind::MacroUse => Some(binding),
ImportKind::Glob { .. } | ImportKind::MacroUse | ImportKind::MacroExport => {
Some(binding)
}
ImportKind::ExternCrate { .. } => None,
},
_ => None,
Expand Down
60 changes: 36 additions & 24 deletions compiler/rustc_resolve/src/effective_visibilities.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,26 +57,45 @@ impl<'r, 'a> EffectiveVisibilitiesVisitor<'r, 'a> {
while let NameBindingKind::Import { binding: nested_binding, import, .. } =
binding.kind
{
let mut update = |node_id| self.update(
self.r.local_def_id(node_id),
binding.vis.expect_local(),
prev_parent_id,
level,
);
// In theory all the import IDs have individual visibilities and effective
// visibilities, but in practice these IDs go straigth to HIR where all
// their few uses assume that their (effective) visibility applies to the
// whole syntactic `use` item. So we update them all to the maximum value
// among the potential individual effective visibilities. Maybe HIR for
// imports shouldn't use three IDs at all.
update(import.id);
if let ImportKind::Single { additional_ids, .. } = import.kind {
update(additional_ids.0);
update(additional_ids.1);
let mut update = |node_id| {
self.update(
self.r.local_def_id(node_id),
binding.vis.expect_local(),
prev_parent_id,
level,
)
};
match import.kind {
ImportKind::Single { id, additional_ids, .. } => {
// In theory all the import IDs have individual visibilities and
// effective visibilities, but in practice these IDs go straigth to
// HIR where all their few uses assume that their (effective)
// visibility applies to the whole syntactic `use` item. So we
// update them all to the maximum value among the potential
// individual effective visibilities. Maybe HIR for imports
// shouldn't use three IDs at all.
update(id);
update(additional_ids.0);
update(additional_ids.1);
prev_parent_id = self.r.local_def_id(id);
}
ImportKind::Glob { id, .. } | ImportKind::ExternCrate { id, .. } => {
update(id);
prev_parent_id = self.r.local_def_id(id);
}
ImportKind::MacroUse => {
// In theory we should reset the parent id to something private
// here, but `macro_use` imports always refer to external items,
// so it doesn't matter and we can just do nothing.
}
ImportKind::MacroExport => {
// In theory we should reset the parent id to something public
// here, but it has the same effect as leaving the previous parent,
// so we can just do nothing.
}
}

level = Level::Reexported;
prev_parent_id = self.r.local_def_id(import.id);
binding = nested_binding;
}
}
Expand Down Expand Up @@ -138,13 +157,6 @@ impl<'r, 'ast> Visitor<'ast> for EffectiveVisibilitiesVisitor<'ast, 'r> {
self.update(def_id, Visibility::Public, parent_id, Level::Direct);
}

// Only exported `macro_rules!` items are public, but they always are
ast::ItemKind::MacroDef(ref macro_def) if macro_def.macro_rules => {
let parent_id = self.r.local_parent(def_id);
let vis = self.r.visibilities[&def_id];
self.update(def_id, vis, parent_id, Level::Direct);
}

ast::ItemKind::Mod(..) => {
self.set_bindings_effective_visibilities(def_id);
visit::walk_item(self, item);
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_resolve/src/ident.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use crate::late::{
};
use crate::macros::{sub_namespace_match, MacroRulesScope};
use crate::{AmbiguityError, AmbiguityErrorMisc, AmbiguityKind, Determinacy, Finalize};
use crate::{ImportKind, LexicalScopeBinding, Module, ModuleKind, ModuleOrUniformRoot};
use crate::{Import, ImportKind, LexicalScopeBinding, Module, ModuleKind, ModuleOrUniformRoot};
use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, PrivacyError, Res};
use crate::{ResolutionError, Resolver, Scope, ScopeSet, Segment, ToNameBinding, Weak};

Expand Down Expand Up @@ -860,7 +860,11 @@ impl<'a> Resolver<'a> {
}

if !restricted_shadowing && binding.expansion != LocalExpnId::ROOT {
if let NameBindingKind::Res(_, true) = binding.kind {
if let NameBindingKind::Import {
import: Import { kind: ImportKind::MacroExport, .. },
..
} = binding.kind
{
self.macro_expanded_macro_export_errors.insert((path_span, binding.span));
}
}
Expand Down
Loading

0 comments on commit 68afa32

Please sign in to comment.