From 801725a77b17a0641fcf40a310f491d71b059d80 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Tue, 14 Jun 2022 14:58:46 +0900 Subject: [PATCH 1/4] suggest adding a `#[macro_export]` to a private macro --- compiler/rustc_resolve/src/imports.rs | 36 +++++++++++++++---- src/test/ui/privacy/macro-private-reexport.rs | 11 ++++++ .../ui/privacy/macro-private-reexport.stderr | 17 +++++++++ .../uniform-paths/macro-rules.stderr | 8 ++--- 4 files changed, 62 insertions(+), 10 deletions(-) create mode 100644 src/test/ui/privacy/macro-private-reexport.rs create mode 100644 src/test/ui/privacy/macro-private-reexport.stderr diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index de83a3a5932..a457169c74b 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -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; @@ -922,11 +922,35 @@ 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, ¬e_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.session.features_untracked().decl_macro + || !self + .r + .session + .source_map() + .span_to_snippet(binding.span) + .map(|snippet| snippet.starts_with("macro ")) + .unwrap_or(true) => + { + 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(); } } } diff --git a/src/test/ui/privacy/macro-private-reexport.rs b/src/test/ui/privacy/macro-private-reexport.rs new file mode 100644 index 00000000000..5b53a861d25 --- /dev/null +++ b/src/test/ui/privacy/macro-private-reexport.rs @@ -0,0 +1,11 @@ +// edition:2018 + +mod foo { + macro_rules! bar { + () => {}; + } + + pub use bar as _; //~ ERROR `bar` is only public within the crate, and cannot be re-exported outside +} + +fn main() {} diff --git a/src/test/ui/privacy/macro-private-reexport.stderr b/src/test/ui/privacy/macro-private-reexport.stderr new file mode 100644 index 00000000000..af85cbcf3f2 --- /dev/null +++ b/src/test/ui/privacy/macro-private-reexport.stderr @@ -0,0 +1,17 @@ +error[E0364]: `bar` is only public within the crate, and cannot be re-exported outside + --> $DIR/macro-private-reexport.rs:8:13 + | +LL | pub use bar as _; + | ^^^^^^^^ + | +help: consider adding a `#[macro_export]` to the macro in the imported module + --> $DIR/macro-private-reexport.rs:4:5 + | +LL | / macro_rules! bar { +LL | | () => {}; +LL | | } + | |_____^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0364`. diff --git a/src/test/ui/rust-2018/uniform-paths/macro-rules.stderr b/src/test/ui/rust-2018/uniform-paths/macro-rules.stderr index 9e48e26b1df..9f8c928c32c 100644 --- a/src/test/ui/rust-2018/uniform-paths/macro-rules.stderr +++ b/src/test/ui/rust-2018/uniform-paths/macro-rules.stderr @@ -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 From 60a50d02abfa3552c825b8d7690cf73343195499 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Tue, 14 Jun 2022 15:46:11 +0900 Subject: [PATCH 2/4] change edition to 2021 --- src/test/ui/privacy/macro-private-reexport.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/privacy/macro-private-reexport.rs b/src/test/ui/privacy/macro-private-reexport.rs index 5b53a861d25..bc3e6fb5c59 100644 --- a/src/test/ui/privacy/macro-private-reexport.rs +++ b/src/test/ui/privacy/macro-private-reexport.rs @@ -1,4 +1,4 @@ -// edition:2018 +// edition:2021 mod foo { macro_rules! bar { From 0d24405211404f5e6b5a746af1ecd08d3d2b4438 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Wed, 15 Jun 2022 00:31:21 +0900 Subject: [PATCH 3/4] implement `MacroData` --- .../rustc_resolve/src/build_reduced_graph.rs | 35 +++++++++++-------- compiler/rustc_resolve/src/ident.rs | 2 +- compiler/rustc_resolve/src/imports.rs | 11 ++---- compiler/rustc_resolve/src/lib.rs | 10 ++++-- compiler/rustc_resolve/src/macros.rs | 4 +-- 5 files changed, 34 insertions(+), 28 deletions(-) diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index f8fa7a0941d..b0e7679af97 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -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}; @@ -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}; @@ -180,26 +181,32 @@ impl<'a> Resolver<'a> { } } - pub(crate) fn get_macro(&mut self, res: Res) -> Option> { + pub(crate) fn get_macro(&mut self, res: Res) -> Option { 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 { - 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( @@ -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 { diff --git a/compiler/rustc_resolve/src/ident.rs b/compiler/rustc_resolve/src/ident.rs index b25393c3ed8..e934e189f05 100644 --- a/compiler/rustc_resolve/src/ident.rs +++ b/compiler/rustc_resolve/src/ident.rs @@ -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) diff --git a/compiler/rustc_resolve/src/imports.rs b/compiler/rustc_resolve/src/imports.rs index a457169c74b..c6aa57f039d 100644 --- a/compiler/rustc_resolve/src/imports.rs +++ b/compiler/rustc_resolve/src/imports.rs @@ -925,16 +925,9 @@ impl<'a, 'b> ImportResolver<'a, 'b> { 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), _) + NameBindingKind::Res(Res::Def(DefKind::Macro(_), def_id), _) // exclude decl_macro - if !self.r.session.features_untracked().decl_macro - || !self - .r - .session - .source_map() - .span_to_snippet(binding.span) - .map(|snippet| snippet.starts_with("macro ")) - .unwrap_or(true) => + if self.r.get_macro_by_def_id(def_id).macro_rules => { err.span_help( binding.span, diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 92a65fe249f..ac4e23cc04d 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -866,6 +866,12 @@ struct DeriveData { has_derive_copy: bool, } +#[derive(Clone)] +struct MacroData { + ext: Lrc, + macro_rules: bool, +} + /// The main resolver class. /// /// This is the visitor that walks the whole crate. @@ -965,7 +971,7 @@ pub struct Resolver<'a> { registered_attrs: FxHashSet, registered_tools: RegisteredTools, macro_use_prelude: FxHashMap>, - macro_map: FxHashMap>, + macro_map: FxHashMap, dummy_ext_bang: Lrc, dummy_ext_derive: Lrc, non_macro_attr: Lrc, @@ -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 { diff --git a/compiler/rustc_resolve/src/macros.rs b/compiler/rustc_resolve/src/macros.rs index 3fb34cdcd9b..c86c2280d19 100644 --- a/compiler/rustc_resolve/src/macros.rs +++ b/compiler/rustc_resolve/src/macros.rs @@ -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) { @@ -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, From d29915af79f4a372647b67e7d83f1fc4aabbb92a Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Wed, 15 Jun 2022 00:42:10 +0900 Subject: [PATCH 4/4] add a test case for `decl_macro` --- src/test/ui/privacy/macro-private-reexport.rs | 6 ++++++ .../ui/privacy/macro-private-reexport.stderr | 18 +++++++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/test/ui/privacy/macro-private-reexport.rs b/src/test/ui/privacy/macro-private-reexport.rs index bc3e6fb5c59..d0aab528ed4 100644 --- a/src/test/ui/privacy/macro-private-reexport.rs +++ b/src/test/ui/privacy/macro-private-reexport.rs @@ -1,11 +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() {} diff --git a/src/test/ui/privacy/macro-private-reexport.stderr b/src/test/ui/privacy/macro-private-reexport.stderr index af85cbcf3f2..b8768f3612e 100644 --- a/src/test/ui/privacy/macro-private-reexport.stderr +++ b/src/test/ui/privacy/macro-private-reexport.stderr @@ -1,17 +1,29 @@ error[E0364]: `bar` is only public within the crate, and cannot be re-exported outside - --> $DIR/macro-private-reexport.rs:8:13 + --> $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:4:5 + --> $DIR/macro-private-reexport.rs:6:5 | LL | / macro_rules! bar { LL | | () => {}; LL | | } | |_____^ -error: aborting due to previous error +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`.