Skip to content
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

exclude unexported macro bindings from extern crate #119369

Merged
merged 1 commit into from
Jan 22, 2024
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
47 changes: 47 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4604,3 +4604,50 @@ declare_lint! {
reference: "issue #X <https://github.com/rust-lang/rust/issues/X>",
};
}

declare_lint! {
/// The `private_macro_use` lint detects private macros that are imported
/// with `#[macro_use]`.
///
/// ### Example
///
/// ```rust,ignore (needs extern crate)
/// // extern_macro.rs
/// macro_rules! foo_ { () => {}; }
/// use foo_ as foo;
///
/// // code.rs
///
/// #![deny(private_macro_use)]
///
/// #[macro_use]
/// extern crate extern_macro;
///
/// fn main() {
/// foo!();
/// }
/// ```
///
/// This will produce:
///
/// ```text
/// error: cannot find macro `foo` in this scope
/// ```
///
/// ### Explanation
///
/// This lint arises from overlooking visibility checks for macros
/// in an external crate.
///
/// This is a [future-incompatible] lint to transition this to a
/// hard error in the future.
///
/// [future-incompatible]: ../index.md#future-incompatible-lints
pub PRIVATE_MACRO_USE,
Warn,
"detects certain macro bindings that should not be re-exported",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps,
reference: "issue #120192 <https://github.com/rust-lang/rust/issues/120192>",
};
}
24 changes: 19 additions & 5 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1029,9 +1029,9 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
}
}

let macro_use_import = |this: &Self, span| {
let macro_use_import = |this: &Self, span, warn_private| {
this.r.arenas.alloc_import(ImportData {
kind: ImportKind::MacroUse,
kind: ImportKind::MacroUse { warn_private },
root_id: item.id,
parent_scope: this.parent_scope,
imported_module: Cell::new(Some(ModuleOrUniformRoot::Module(module))),
Expand All @@ -1048,11 +1048,25 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {

let allow_shadowing = self.parent_scope.expansion == LocalExpnId::ROOT;
if let Some(span) = import_all {
let import = macro_use_import(self, span);
let import = macro_use_import(self, span, false);
self.r.potentially_unused_imports.push(import);
module.for_each_child(self, |this, ident, ns, binding| {
if ns == MacroNS {
let imported_binding = this.r.import(binding, import);
let imported_binding =
if this.r.is_accessible_from(binding.vis, this.parent_scope.module) {
this.r.import(binding, import)
} else if !this.r.is_builtin_macro(binding.res())
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
&& !this.r.macro_use_prelude.contains_key(&ident.name)
{
// - `!r.is_builtin_macro(res)` excluding the built-in macros such as `Debug` or `Hash`.
// - `!r.macro_use_prelude.contains_key(name)` excluding macros defined in other extern
// crates such as `std`.
// FIXME: This branch should eventually be removed.
let import = macro_use_import(this, span, true);
this.r.import(binding, import)
} else {
return;
};
this.add_macro_use_binding(ident.name, imported_binding, span, allow_shadowing);
}
});
Expand All @@ -1065,7 +1079,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
&self.parent_scope,
);
if let Ok(binding) = result {
let import = macro_use_import(self, ident.span);
let import = macro_use_import(self, ident.span, false);
self.r.potentially_unused_imports.push(import);
let imported_binding = self.r.import(binding, import);
self.add_macro_use_binding(
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_resolve/src/check_unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ impl Resolver<'_, '_> {
|| import.expect_vis().is_public()
|| import.span.is_dummy() =>
{
if let ImportKind::MacroUse = import.kind {
if let ImportKind::MacroUse { .. } = import.kind {
if !import.span.is_dummy() {
self.lint_buffer.buffer_lint(
MACRO_USE_EXTERN_CRATE,
Expand All @@ -315,7 +315,7 @@ impl Resolver<'_, '_> {
maybe_unused_extern_crates.insert(id, import.span);
}
}
ImportKind::MacroUse => {
ImportKind::MacroUse { .. } => {
let msg = "unused `#[macro_use]` import";
self.lint_buffer.buffer_lint(UNUSED_IMPORTS, import.root_id, import.span, msg);
}
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
use NameBindingKind::Import;
let can_suggest = |binding: NameBinding<'_>, import: self::Import<'_>| {
!binding.span.is_dummy()
&& !matches!(import.kind, ImportKind::MacroUse | ImportKind::MacroExport)
&& !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
Expand Down Expand Up @@ -1819,9 +1819,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
next_ident = source;
Some(binding)
}
ImportKind::Glob { .. } | ImportKind::MacroUse | ImportKind::MacroExport => {
Some(binding)
}
ImportKind::Glob { .. }
| ImportKind::MacroUse { .. }
| ImportKind::MacroExport => Some(binding),
ImportKind::ExternCrate { .. } => None,
},
_ => None,
Expand Down
14 changes: 9 additions & 5 deletions compiler/rustc_resolve/src/imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@ pub(crate) enum ImportKind<'a> {
target: Ident,
id: NodeId,
},
MacroUse,
MacroUse {
/// A field has been added indicating whether it should be reported as a lint,
/// addressing issue#119301.
warn_private: bool,
},
MacroExport,
}

Expand Down Expand Up @@ -127,7 +131,7 @@ impl<'a> std::fmt::Debug for ImportKind<'a> {
.field("target", target)
.field("id", id)
.finish(),
MacroUse => f.debug_struct("MacroUse").finish(),
MacroUse { .. } => f.debug_struct("MacroUse").finish(),
MacroExport => f.debug_struct("MacroExport").finish(),
}
}
Expand Down Expand Up @@ -197,7 +201,7 @@ impl<'a> ImportData<'a> {
ImportKind::Single { id, .. }
| ImportKind::Glob { id, .. }
| ImportKind::ExternCrate { id, .. } => Some(id),
ImportKind::MacroUse | ImportKind::MacroExport => None,
ImportKind::MacroUse { .. } | ImportKind::MacroExport => None,
}
}

Expand All @@ -207,7 +211,7 @@ impl<'a> ImportData<'a> {
ImportKind::Single { id, .. } => Reexport::Single(to_def_id(id)),
ImportKind::Glob { id, .. } => Reexport::Glob(to_def_id(id)),
ImportKind::ExternCrate { id, .. } => Reexport::ExternCrate(to_def_id(id)),
ImportKind::MacroUse => Reexport::MacroUse,
ImportKind::MacroUse { .. } => Reexport::MacroUse,
ImportKind::MacroExport => Reexport::MacroExport,
}
}
Expand Down Expand Up @@ -1482,7 +1486,7 @@ fn import_kind_to_string(import_kind: &ImportKind<'_>) -> String {
ImportKind::Single { source, .. } => source.to_string(),
ImportKind::Glob { .. } => "*".to_string(),
ImportKind::ExternCrate { .. } => "<extern crate>".to_string(),
ImportKind::MacroUse => "#[macro_use]".to_string(),
ImportKind::MacroUse { .. } => "#[macro_use]".to_string(),
ImportKind::MacroExport => "#[macro_export]".to_string(),
}
}
5 changes: 5 additions & 0 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ use rustc_middle::span_bug;
use rustc_middle::ty::{self, MainDefinition, RegisteredTools, TyCtxt};
use rustc_middle::ty::{ResolverGlobalCtxt, ResolverOutputs};
use rustc_query_system::ich::StableHashingContext;
use rustc_session::lint::builtin::PRIVATE_MACRO_USE;
use rustc_session::lint::LintBuffer;
use rustc_span::hygiene::{ExpnId, LocalExpnId, MacroKind, SyntaxContext, Transparency};
use rustc_span::symbol::{kw, sym, Ident, Symbol};
Expand Down Expand Up @@ -1799,6 +1800,10 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
}
}
if let NameBindingKind::Import { import, binding, ref used } = used_binding.kind {
if let ImportKind::MacroUse { warn_private: true } = import.kind {
let msg = format!("macro `{ident}` is private");
self.lint_buffer().buffer_lint(PRIVATE_MACRO_USE, import.root_id, ident.span, msg);
}
// Avoid marking `extern crate` items that refer to a name from extern prelude,
// but not introduce it, as used if they are accessed from lexical scope.
if is_lexical_scope {
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/extern/auxiliary/issue-80074-macro-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// edition:2018

macro_rules! m { () => {}; }
2 changes: 2 additions & 0 deletions tests/ui/extern/auxiliary/issue-80074-macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@

macro_rules! foo_ { () => {}; }
use foo_ as foo;

macro_rules! bar { () => {}; }
12 changes: 11 additions & 1 deletion tests/ui/extern/issue-80074.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,20 @@
// edition:2018
// build-pass
// aux-crate:issue_80074=issue-80074-macro.rs
// aux-crate:issue_80074_2=issue-80074-macro-2.rs

#[macro_use]
extern crate issue_80074;

#[macro_use(m)]
extern crate issue_80074_2;
//~^^ ERROR: imported macro not found

fn main() {
foo!();
//~^ WARN: macro `foo` is private
//~| WARN: it will become a hard error in a future release!
bar!();
//~^ ERROR: cannot find macro `bar` in this scope
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
m!();
//~^ ERROR: cannot find macro `m` in this scope
}
31 changes: 31 additions & 0 deletions tests/ui/extern/issue-80074.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
error[E0469]: imported macro not found
--> $DIR/issue-80074.rs:8:13
|
LL | #[macro_use(m)]
| ^

error: cannot find macro `bar` in this scope
--> $DIR/issue-80074.rs:16:5
|
LL | bar!();
| ^^^

error: cannot find macro `m` in this scope
--> $DIR/issue-80074.rs:18:5
|
LL | m!();
| ^

warning: macro `foo` is private
--> $DIR/issue-80074.rs:13:5
|
LL | foo!();
| ^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #120192 <https://github.com/rust-lang/rust/issues/120192>
= note: `#[warn(private_macro_use)]` on by default

error: aborting due to 3 previous errors; 1 warning emitted

For more information about this error, try `rustc --explain E0469`.
2 changes: 2 additions & 0 deletions tests/ui/imports/auxiliary/issue-119369-extern.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
use std::vec;
use std::hash::Hash;
14 changes: 14 additions & 0 deletions tests/ui/imports/issue-119369.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// check-pass
// aux-build: issue-119369-extern.rs

// https://github.com/rust-lang/rust/pull/119369#issuecomment-1874905662

#[macro_use]
extern crate issue_119369_extern;

#[derive(Hash)]
struct A;

fn main() {
let _: Vec<i32> = vec![];
}
Loading