Skip to content

resolve: Turn the binding from #[macro_export] into a proper Import #103760

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.

Sign up for GitHub

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 2 commits into from
Nov 1, 2022
+237 −164
Merged

resolve: Turn the binding from #[macro_export] into a proper Import #103760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
637bfe6
resolve: Not all imports have their own `NodeId`
petrochenkov Oct 30, 2022
8431751
resolve: Turn the binding from `#[macro_export]` into a proper `Import`
petrochenkov Oct 30, 2022
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
55 changes: 22 additions & 33 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,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 @@ -915,7 +915,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