Skip to content

Commit

Permalink
Unrolled build for rust-lang#119369
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#119369 - bvanjoi:fix-119301, r=petrochenkov

exclude unexported macro bindings from extern crate

Fixes rust-lang#119301

Macros that aren't exported from an external crate should not be defined.

r? ``@petrochenkov``
  • Loading branch information
rust-timer authored Jan 22, 2024
2 parents 3066253 + 9c3091e commit 631b316
Show file tree
Hide file tree
Showing 12 changed files with 149 additions and 17 deletions.
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())
&& !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
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![];
}

0 comments on commit 631b316

Please sign in to comment.