Skip to content

Commit a3883c7

Browse files
bvanjoibvanjoi
committed
exclude unexported macro bindings from extern crate
1 parent c485ee7 commit a3883c7

File tree

12 files changed

+153
-17
lines changed

12 files changed

+153
-17
lines changed

‎compiler/rustc_lint_defs/src/builtin.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4604,3 +4604,49 @@ declare_lint! {
46044604
reference: "issue #X <https://github.com/rust-lang/rust/issues/X>",
46054605
};
46064606
}
4607+
4608+
declare_lint! {
4609+
/// The `private_macro_use` lint detects macros that are imported
4610+
/// with `#[macro_use]`.
4611+
///
4612+
/// ### Example
4613+
///
4614+
/// ```rust,ignore (needs extern crate)
4615+
/// // extern_macro.rs
4616+
/// macro_rules! foo { () => {}; }
4617+
///
4618+
/// // code.rs
4619+
///
4620+
/// #![deny(private_macro_use)]
4621+
///
4622+
/// #[macro_use]
4623+
/// extern crate extern_macro;
4624+
///
4625+
/// fn main() {
4626+
/// foo!();
4627+
/// }
4628+
/// ```
4629+
///
4630+
/// This will produce:
4631+
///
4632+
/// ```text
4633+
/// error: cannot find macro `foo` in this scope
4634+
/// ```
4635+
///
4636+
/// ### Explanation
4637+
///
4638+
/// This lint arises from overlooking visibility checks for macros
4639+
/// in an external crate.
4640+
///
4641+
/// This is a [future-incompatible] lint to transition this to a
4642+
/// hard error in the future.
4643+
///
4644+
/// [future-incompatible]: ../index.md#future-incompatible-lints
4645+
pub PRIVATE_MACRO_USE,
4646+
Warn,
4647+
"detects certain macro bindings that should not be re-exported",
4648+
@future_incompatible = FutureIncompatibleInfo {
4649+
reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps,
4650+
reference: "issue #X <https://github.com/rust-lang/rust/issues/X>",
4651+
};
4652+
}

‎compiler/rustc_resolve/src/build_reduced_graph.rs

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,9 +1029,9 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
10291029
}
10301030
}
10311031

1032-
let macro_use_import = |this: &Self, span| {
1032+
let macro_use_import = |this: &Self, span: Span, warn_not_exported: bool| {
10331033
this.r.arenas.alloc_import(ImportData {
1034-
kind: ImportKind::MacroUse,
1034+
kind: ImportKind::MacroUse { warn_not_exported },
10351035
root_id: item.id,
10361036
parent_scope: this.parent_scope,
10371037
imported_module: Cell::new(Some(ModuleOrUniformRoot::Module(module))),
@@ -1048,11 +1048,28 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
10481048

10491049
let allow_shadowing = self.parent_scope.expansion == LocalExpnId::ROOT;
10501050
if let Some(span) = import_all {
1051-
let import = macro_use_import(self, span);
1051+
let import = macro_use_import(self, span, false);
10521052
self.r.potentially_unused_imports.push(import);
10531053
module.for_each_child(self, |this, ident, ns, binding| {
10541054
if ns == MacroNS {
1055-
let imported_binding = this.r.import(binding, import);
1055+
let imported_binding = if this
1056+
.r
1057+
.is_accessible_from(binding.vis, this.parent_scope.module)
1058+
{
1059+
this.r.import(binding, import)
1060+
} else if !this.r.is_builtin_macro(binding.res())
1061+
&& !this.r.macro_use_prelude.contains_key(&ident.name)
1062+
{
1063+
// - `!r.is_builtin_macro(res)` excluding the built-in macros such as `Debug` or `Hash`.
1064+
// - `!r.macro_use_prelude.contains_key(name)` excluding macros defined in other extern
1065+
// crates such as `std`.
1066+
// FIXME: This branch should eventually be removed.
1067+
let import = macro_use_import(this, span, true);
1068+
let binding = this.r.import(binding, import);
1069+
self.r.arenas.alloc_name_binding(NameBindingData { ..(*binding).clone() })
1070+
} else {
1071+
return;
1072+
};
10561073
this.add_macro_use_binding(ident.name, imported_binding, span, allow_shadowing);
10571074
}
10581075
});
@@ -1065,7 +1082,7 @@ impl<'a, 'b, 'tcx> BuildReducedGraphVisitor<'a, 'b, 'tcx> {
10651082
&self.parent_scope,
10661083
);
10671084
if let Ok(binding) = result {
1068-
let import = macro_use_import(self, ident.span);
1085+
let import = macro_use_import(self, ident.span, false);
10691086
self.r.potentially_unused_imports.push(import);
10701087
let imported_binding = self.r.import(binding, import);
10711088
self.add_macro_use_binding(

‎compiler/rustc_resolve/src/check_unused.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ impl Resolver<'_, '_> {
290290
|| import.expect_vis().is_public()
291291
|| import.span.is_dummy() =>
292292
{
293-
if let ImportKind::MacroUse = import.kind {
293+
if let ImportKind::MacroUse { .. } = import.kind {
294294
if !import.span.is_dummy() {
295295
self.lint_buffer.buffer_lint(
296296
MACRO_USE_EXTERN_CRATE,
@@ -315,7 +315,7 @@ impl Resolver<'_, '_> {
315315
maybe_unused_extern_crates.insert(id, import.span);
316316
}
317317
}
318-
ImportKind::MacroUse => {
318+
ImportKind::MacroUse { .. } => {
319319
let msg = "unused `#[macro_use]` import";
320320
self.lint_buffer.buffer_lint(UNUSED_IMPORTS, import.root_id, import.span, msg);
321321
}

‎compiler/rustc_resolve/src/diagnostics.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
285285
use NameBindingKind::Import;
286286
let can_suggest = |binding: NameBinding<'_>, import: self::Import<'_>| {
287287
!binding.span.is_dummy()
288-
&& !matches!(import.kind, ImportKind::MacroUse | ImportKind::MacroExport)
288+
&& !matches!(import.kind, ImportKind::MacroUse { .. } | ImportKind::MacroExport)
289289
};
290290
let import = match (&new_binding.kind, &old_binding.kind) {
291291
// If there are two imports where one or both have attributes then prefer removing the
@@ -1819,9 +1819,9 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
18191819
next_ident = source;
18201820
Some(binding)
18211821
}
1822-
ImportKind::Glob { .. } | ImportKind::MacroUse | ImportKind::MacroExport => {
1823-
Some(binding)
1824-
}
1822+
ImportKind::Glob { .. }
1823+
| ImportKind::MacroUse { .. }
1824+
| ImportKind::MacroExport => Some(binding),
18251825
ImportKind::ExternCrate { .. } => None,
18261826
},
18271827
_ => None,

‎compiler/rustc_resolve/src/imports.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,11 @@ pub(crate) enum ImportKind<'a> {
8080
target: Ident,
8181
id: NodeId,
8282
},
83-
MacroUse,
83+
MacroUse {
84+
/// A field has been added indicating whether it should be reported as a lint,
85+
/// addressing issue#119301.
86+
warn_not_exported: bool,
87+
},
8488
MacroExport,
8589
}
8690

@@ -127,7 +131,7 @@ impl<'a> std::fmt::Debug for ImportKind<'a> {
127131
.field("target", target)
128132
.field("id", id)
129133
.finish(),
130-
MacroUse => f.debug_struct("MacroUse").finish(),
134+
MacroUse { .. } => f.debug_struct("MacroUse").finish(),
131135
MacroExport => f.debug_struct("MacroExport").finish(),
132136
}
133137
}
@@ -197,7 +201,7 @@ impl<'a> ImportData<'a> {
197201
ImportKind::Single { id, .. }
198202
| ImportKind::Glob { id, .. }
199203
| ImportKind::ExternCrate { id, .. } => Some(id),
200-
ImportKind::MacroUse | ImportKind::MacroExport => None,
204+
ImportKind::MacroUse { .. } | ImportKind::MacroExport => None,
201205
}
202206
}
203207

@@ -207,7 +211,7 @@ impl<'a> ImportData<'a> {
207211
ImportKind::Single { id, .. } => Reexport::Single(to_def_id(id)),
208212
ImportKind::Glob { id, .. } => Reexport::Glob(to_def_id(id)),
209213
ImportKind::ExternCrate { id, .. } => Reexport::ExternCrate(to_def_id(id)),
210-
ImportKind::MacroUse => Reexport::MacroUse,
214+
ImportKind::MacroUse { .. } => Reexport::MacroUse,
211215
ImportKind::MacroExport => Reexport::MacroExport,
212216
}
213217
}
@@ -1482,7 +1486,7 @@ fn import_kind_to_string(import_kind: &ImportKind<'_>) -> String {
14821486
ImportKind::Single { source, .. } => source.to_string(),
14831487
ImportKind::Glob { .. } => "*".to_string(),
14841488
ImportKind::ExternCrate { .. } => "<extern crate>".to_string(),
1485-
ImportKind::MacroUse => "#[macro_use]".to_string(),
1489+
ImportKind::MacroUse { .. } => "#[macro_use]".to_string(),
14861490
ImportKind::MacroExport => "#[macro_export]".to_string(),
14871491
}
14881492
}

‎compiler/rustc_resolve/src/lib.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ use rustc_middle::span_bug;
5555
use rustc_middle::ty::{self, MainDefinition, RegisteredTools, TyCtxt};
5656
use rustc_middle::ty::{ResolverGlobalCtxt, ResolverOutputs};
5757
use rustc_query_system::ich::StableHashingContext;
58+
use rustc_session::lint::builtin::PRIVATE_MACRO_USE;
5859
use rustc_session::lint::LintBuffer;
5960
use rustc_span::hygiene::{ExpnId, LocalExpnId, MacroKind, SyntaxContext, Transparency};
6061
use rustc_span::symbol::{kw, sym, Ident, Symbol};
@@ -1808,6 +1809,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
18081809
}
18091810
}
18101811
}
1812+
if let ImportKind::MacroUse { warn_not_exported } = import.kind
1813+
&& warn_not_exported
1814+
{
1815+
let msg = format!("macro `{ident}` is private");
1816+
self.lint_buffer().buffer_lint(PRIVATE_MACRO_USE, import.root_id, ident.span, msg);
1817+
}
18111818
used.set(true);
18121819
import.used.set(true);
18131820
if let Some(id) = import.id() {

‎tests/ui/extern/auxiliary/issue-80074-macro-2.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// edition:2018
2+
3+
macro_rules! m { () => {}; }

‎tests/ui/extern/auxiliary/issue-80074-macro.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,5 @@
22

33
macro_rules! foo_ { () => {}; }
44
use foo_ as foo;
5+
6+
macro_rules! bar { () => {}; }

‎tests/ui/extern/issue-80074.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,20 @@
11
// edition:2018
2-
// build-pass
32
// aux-crate:issue_80074=issue-80074-macro.rs
3+
// aux-crate:issue_80074_2=issue-80074-macro-2.rs
44

55
#[macro_use]
66
extern crate issue_80074;
77

8+
#[macro_use(m)]
9+
extern crate issue_80074_2;
10+
//~^^ ERROR: imported macro not found
11+
812
fn main() {
913
foo!();
14+
//~^ WARN: macro `foo` is private
15+
//~| WARN: it will become a hard error in a future release!
16+
bar!();
17+
//~^ ERROR: cannot find macro `bar` in this scope
18+
m!();
19+
//~^ ERROR: cannot find macro `m` in this scope
1020
}

‎tests/ui/extern/issue-80074.stderr

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
error[E0469]: imported macro not found
2+
--> $DIR/issue-80074.rs:8:13
3+
|
4+
LL | #[macro_use(m)]
5+
| ^
6+
7+
error: cannot find macro `bar` in this scope
8+
--> $DIR/issue-80074.rs:16:5
9+
|
10+
LL | bar!();
11+
| ^^^
12+
13+
error: cannot find macro `m` in this scope
14+
--> $DIR/issue-80074.rs:18:5
15+
|
16+
LL | m!();
17+
| ^
18+
19+
warning: macro `foo` is private
20+
--> $DIR/issue-80074.rs:13:5
21+
|
22+
LL | foo!();
23+
| ^^^
24+
|
25+
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
26+
= note: for more information, see issue #X <https://github.com/rust-lang/rust/issues/X>
27+
= note: `#[warn(private_macro_use)]` on by default
28+
29+
error: aborting due to 3 previous errors; 1 warning emitted
30+
31+
For more information about this error, try `rustc --explain E0469`.

‎tests/ui/imports/auxiliary/issue-119369-extern.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
use std::vec;
2+
use std::hash::Hash;

‎tests/ui/imports/issue-119369.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// check-pass
2+
// aux-build: issue-119369-extern.rs
3+
4+
// https://github.com/rust-lang/rust/pull/119369#issuecomment-1874905662
5+
6+
#[macro_use]
7+
extern crate issue_119369_extern;
8+
9+
#[derive(Hash)]
10+
struct A;
11+
12+
fn main() {
13+
let _: Vec<i32> = vec![];
14+
}

0 commit comments

Comments
 (0)