Skip to content

Suggest adding a #[macro_export] to a private macro #98087

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.

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 4 commits into from
Jun 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 21 additions & 14 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ use crate::imports::{Import, ImportKind};
use crate::macros::{MacroRulesBinding, MacroRulesScope, MacroRulesScopeRef};
use crate::Namespace::{self, MacroNS, TypeNS, ValueNS};
use crate::{Determinacy, ExternPreludeEntry, Finalize, Module, ModuleKind, ModuleOrUniformRoot};
use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, PerNS, ResolutionError};
use crate::{
MacroData, NameBinding, NameBindingKind, ParentScope, PathResult, PerNS, ResolutionError,
};
use crate::{Resolver, ResolverArenas, Segment, ToNameBinding, VisResolutionError};

use rustc_ast::visit::{self, AssocCtxt, Visitor};
Expand All @@ -20,7 +22,6 @@ use rustc_ast_lowering::ResolverAstLowering;
use rustc_attr as attr;
use rustc_data_structures::sync::Lrc;
use rustc_errors::{struct_span_err, Applicability};
use rustc_expand::base::SyntaxExtension;
use rustc_expand::expand::AstFragment;
use rustc_hir::def::{self, *};
use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID};
Expand Down Expand Up @@ -180,26 +181,32 @@ impl<'a> Resolver<'a> {
}
}

pub(crate) fn get_macro(&mut self, res: Res) -> Option<Lrc<SyntaxExtension>> {
pub(crate) fn get_macro(&mut self, res: Res) -> Option<MacroData> {
match res {
Res::Def(DefKind::Macro(..), def_id) => Some(self.get_macro_by_def_id(def_id)),
Res::NonMacroAttr(_) => Some(self.non_macro_attr.clone()),
Res::NonMacroAttr(_) => {
Some(MacroData { ext: self.non_macro_attr.clone(), macro_rules: false })
}
_ => None,
}
}

pub(crate) fn get_macro_by_def_id(&mut self, def_id: DefId) -> Lrc<SyntaxExtension> {
if let Some(ext) = self.macro_map.get(&def_id) {
return ext.clone();
pub(crate) fn get_macro_by_def_id(&mut self, def_id: DefId) -> MacroData {
if let Some(macro_data) = self.macro_map.get(&def_id) {
return macro_data.clone();
}

let ext = Lrc::new(match self.cstore().load_macro_untracked(def_id, &self.session) {
LoadedMacro::MacroDef(item, edition) => self.compile_macro(&item, edition).0,
LoadedMacro::ProcMacro(ext) => ext,
});
let (ext, macro_rules) = match self.cstore().load_macro_untracked(def_id, &self.session) {
LoadedMacro::MacroDef(item, edition) => (
Lrc::new(self.compile_macro(&item, edition).0),
matches!(item.kind, ItemKind::MacroDef(def) if def.macro_rules),
),
LoadedMacro::ProcMacro(extz) => (Lrc::new(extz), false),
};

self.macro_map.insert(def_id, ext.clone());
ext
let macro_data = MacroData { ext, macro_rules };
self.macro_map.insert(def_id, macro_data.clone());
macro_data
}

pub(crate) fn build_reduced_graph(
Expand Down Expand Up @@ -1251,7 +1258,7 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
};

let res = Res::Def(DefKind::Macro(ext.macro_kind()), def_id.to_def_id());
self.r.macro_map.insert(def_id.to_def_id(), ext);
self.r.macro_map.insert(def_id.to_def_id(), MacroData { ext, macro_rules });
self.r.local_macro_def_scopes.insert(def_id, parent_scope.module);

if macro_rules {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ impl<'a> Resolver<'a> {
{
// The macro is a proc macro derive
if let Some(def_id) = module.expansion.expn_data().macro_def_id {
let ext = self.get_macro_by_def_id(def_id);
let ext = self.get_macro_by_def_id(def_id).ext;
if ext.builtin_name.is_none()
&& ext.macro_kind() == MacroKind::Derive
&& parent.expansion.outer_expn_is_descendant_of(*ctxt)
Expand Down
29 changes: 23 additions & 6 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_ast::NodeId;
use rustc_data_structures::fx::FxHashSet;
use rustc_data_structures::intern::Interned;
use rustc_errors::{pluralize, struct_span_err, Applicability, MultiSpan};
use rustc_hir::def::{self, PartialRes};
use rustc_hir::def::{self, DefKind, PartialRes};
use rustc_middle::metadata::ModChild;
use rustc_middle::span_bug;
use rustc_middle::ty;
Expand Down Expand Up @@ -922,11 +922,28 @@ impl<'a, 'b> ImportResolver<'a, 'b> {
.note(&format!("consider declaring type or module `{}` with `pub`", ident))
.emit();
} else {
let note_msg =
format!("consider marking `{}` as `pub` in the imported module", ident);
struct_span_err!(self.r.session, import.span, E0364, "{}", error_msg)
.span_note(import.span, &note_msg)
.emit();
let mut err =
struct_span_err!(self.r.session, import.span, E0364, "{error_msg}");
match binding.kind {
NameBindingKind::Res(Res::Def(DefKind::Macro(_), def_id), _)
// exclude decl_macro
if self.r.get_macro_by_def_id(def_id).macro_rules =>
{
err.span_help(
binding.span,
"consider adding a `#[macro_export]` to the macro in the imported module",
);
}
_ => {
err.span_note(
import.span,
&format!(
"consider marking `{ident}` as `pub` in the imported module"
),
);
}
}
err.emit();
}
}
}
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -866,6 +866,12 @@ struct DeriveData {
has_derive_copy: bool,
}

#[derive(Clone)]
struct MacroData {
ext: Lrc<SyntaxExtension>,
macro_rules: bool,
}

/// The main resolver class.
///
/// This is the visitor that walks the whole crate.
Expand Down Expand Up @@ -965,7 +971,7 @@ pub struct Resolver<'a> {
registered_attrs: FxHashSet<Ident>,
registered_tools: RegisteredTools,
macro_use_prelude: FxHashMap<Symbol, &'a NameBinding<'a>>,
macro_map: FxHashMap<DefId, Lrc<SyntaxExtension>>,
macro_map: FxHashMap<DefId, MacroData>,
dummy_ext_bang: Lrc<SyntaxExtension>,
dummy_ext_derive: Lrc<SyntaxExtension>,
non_macro_attr: Lrc<SyntaxExtension>,
Expand Down Expand Up @@ -1522,7 +1528,7 @@ impl<'a> Resolver<'a> {
}

fn is_builtin_macro(&mut self, res: Res) -> bool {
self.get_macro(res).map_or(false, |ext| ext.builtin_name.is_some())
self.get_macro(res).map_or(false, |macro_data| macro_data.ext.builtin_name.is_some())
}

fn macro_def(&self, mut ctxt: SyntaxContext) -> DefId {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_resolve/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ impl<'a> Resolver<'a> {
res
};

res.map(|res| (self.get_macro(res), res))
res.map(|res| (self.get_macro(res).map(|macro_data| macro_data.ext), res))
}

pub(crate) fn finalize_macro_resolutions(&mut self) {
Expand Down Expand Up @@ -853,7 +853,7 @@ impl<'a> Resolver<'a> {
// Reserve some names that are not quite covered by the general check
// performed on `Resolver::builtin_attrs`.
if ident.name == sym::cfg || ident.name == sym::cfg_attr {
let macro_kind = self.get_macro(res).map(|ext| ext.macro_kind());
let macro_kind = self.get_macro(res).map(|macro_data| macro_data.ext.macro_kind());
if macro_kind.is_some() && sub_namespace_match(macro_kind, Some(MacroKind::Attr)) {
self.session.span_err(
ident.span,
Expand Down
17 changes: 17 additions & 0 deletions src/test/ui/privacy/macro-private-reexport.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// edition:2021

#![feature(decl_macro)]

mod foo {
macro_rules! bar {
() => {};
}

pub use bar as _; //~ ERROR `bar` is only public within the crate, and cannot be re-exported outside

macro baz() {}

pub use baz as _; //~ ERROR `baz` is private, and cannot be re-exported
}

fn main() {}
29 changes: 29 additions & 0 deletions src/test/ui/privacy/macro-private-reexport.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
error[E0364]: `bar` is only public within the crate, and cannot be re-exported outside
--> $DIR/macro-private-reexport.rs:10:13
|
LL | pub use bar as _;
| ^^^^^^^^
|
help: consider adding a `#[macro_export]` to the macro in the imported module
--> $DIR/macro-private-reexport.rs:6:5
|
LL | / macro_rules! bar {
LL | | () => {};
LL | | }
| |_____^

error[E0364]: `baz` is private, and cannot be re-exported
--> $DIR/macro-private-reexport.rs:14:13
|
LL | pub use baz as _;
| ^^^^^^^^
|
note: consider marking `baz` as `pub` in the imported module
--> $DIR/macro-private-reexport.rs:14:13
|
LL | pub use baz as _;
| ^^^^^^^^

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0364`.
8 changes: 4 additions & 4 deletions src/test/ui/rust-2018/uniform-paths/macro-rules.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ error[E0364]: `legacy_macro` is only public within the crate, and cannot be re-e
LL | pub use legacy_macro as _;
| ^^^^^^^^^^^^^^^^^
|
note: consider marking `legacy_macro` as `pub` in the imported module
--> $DIR/macro-rules.rs:11:13
help: consider adding a `#[macro_export]` to the macro in the imported module
--> $DIR/macro-rules.rs:7:5
|
LL | pub use legacy_macro as _;
| ^^^^^^^^^^^^^^^^^
LL | macro_rules! legacy_macro { () => () }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error[E0659]: `legacy_macro` is ambiguous
--> $DIR/macro-rules.rs:31:13
Expand Down