Skip to content

Commit c4c7859

Browse files
petrochenkovpetrochenkov
committed
resolve: Implement a lint for out-of-scope use of macro_rules 
1 parent 0195758 commit c4c7859

12 files changed

+273
-92
lines changed

‎compiler/rustc_lint/messages.ftl

+3
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,9 @@ lint_opaque_hidden_inferred_bound_sugg = add this bound
604604
lint_or_patterns_back_compat = the meaning of the `pat` fragment specifier is changing in Rust 2021, which may affect this macro
605605
.suggestion = use pat_param to preserve semantics
606606
607+
lint_out_of_scope_macro_calls = cannot find macro `{$path}` in this scope
608+
.help = import `macro_rules` with `use` to make it callable above its definition
609+
607610
lint_overflowing_bin_hex = literal out of range for `{$ty}`
608611
.negative_note = the literal `{$lit}` (decimal `{$dec}`) does not fit into the type `{$ty}`
609612
.negative_becomes_note = and the value `-{$lit}` will become `{$actually}{$ty}`

‎compiler/rustc_lint/src/context/diagnostics.rs

+3
Original file line numberDiff line numberDiff line change
@@ -434,5 +434,8 @@ pub(super) fn decorate_lint(sess: &Session, diagnostic: BuiltinLintDiag, diag: &
434434
lints::InnerAttributeUnstable::CustomInnerAttribute
435435
}
436436
.decorate_lint(diag),
437+
BuiltinLintDiag::OutOfScopeMacroCalls { path } => {
438+
lints::OutOfScopeMacroCalls { path }.decorate_lint(diag)
439+
}
437440
}
438441
}

‎compiler/rustc_lint/src/lints.rs

+7
Original file line numberDiff line numberDiff line change
@@ -2911,3 +2911,10 @@ pub struct UnsafeAttrOutsideUnsafeSuggestion {
29112911
#[suggestion_part(code = ")")]
29122912
pub right: Span,
29132913
}
2914+
2915+
#[derive(LintDiagnostic)]
2916+
#[diag(lint_out_of_scope_macro_calls)]
2917+
#[help]
2918+
pub struct OutOfScopeMacroCalls {
2919+
pub path: String,
2920+
}

‎compiler/rustc_lint_defs/src/builtin.rs

+39
Original file line numberDiff line numberDiff line change
@@ -4945,3 +4945,42 @@ declare_lint! {
49454945
reference: "issue #123757 <https://github.com/rust-lang/rust/issues/123757>",
49464946
};
49474947
}
4948+
4949+
declare_lint! {
4950+
/// The `out_of_scope_macro_calls` lint detects `macro_rules` called when they are not in scope,
4951+
/// above their definition, which may happen in key-value attributes.
4952+
///
4953+
/// ### Example
4954+
///
4955+
/// ```rust
4956+
/// #![doc = in_root!()]
4957+
///
4958+
/// macro_rules! in_root { () => { "" } }
4959+
///
4960+
/// fn main() {}
4961+
/// ```
4962+
///
4963+
/// {{produces}}
4964+
///
4965+
/// ### Explanation
4966+
///
4967+
/// The scope in which a `macro_rules` item is visible starts at that item and continues
4968+
/// below it. This is more similar to `let` than to other items, which are in scope both above
4969+
/// and below their definition.
4970+
/// Due to a bug `macro_rules` were accidentally in scope inside some key-value attributes
4971+
/// above their definition. The lint catches such cases.
4972+
/// To address the issue turn the `macro_rules` into a regularly scoped item by importing it
4973+
/// with `use`.
4974+
///
4975+
/// This is a [future-incompatible] lint to transition this to a
4976+
/// hard error in the future.
4977+
///
4978+
/// [future-incompatible]: ../index.md#future-incompatible-lints
4979+
pub OUT_OF_SCOPE_MACRO_CALLS,
4980+
Warn,
4981+
"detects out of scope calls to `macro_rules` in key-value attributes",
4982+
@future_incompatible = FutureIncompatibleInfo {
4983+
reason: FutureIncompatibilityReason::FutureReleaseErrorDontReportInDeps,
4984+
reference: "issue #124535 <https://github.com/rust-lang/rust/issues/124535>",
4985+
};
4986+
}

‎compiler/rustc_lint_defs/src/lib.rs

+3
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,9 @@ pub enum BuiltinLintDiag {
744744
InnerAttributeUnstable {
745745
is_macro: bool,
746746
},
747+
OutOfScopeMacroCalls {
748+
path: String,
749+
},
747750
}
748751

749752
/// Lints that are buffered up early on in the `Session` before the

‎compiler/rustc_resolve/src/build_reduced_graph.rs

+16-3
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ use crate::{Determinacy, ExternPreludeEntry, Finalize, Module, ModuleKind, Modul
1414
use crate::{NameBinding, NameBindingKind, ParentScope, PathResult, ResolutionError};
1515
use crate::{Resolver, ResolverArenas, Segment, ToNameBinding, Used, VisResolutionError};
1616

17-
use rustc_ast::visit::{self, AssocCtxt, Visitor};
17+
use rustc_ast::visit::{self, AssocCtxt, Visitor, WalkItemKind};
1818
use rustc_ast::{self as ast, AssocItem, AssocItemKind, MetaItemKind, StmtKind};
1919
use rustc_ast::{Block, ForeignItem, ForeignItemKind, Impl, Item, ItemKind, NodeId};
2020
use rustc_attr as attr;
@@ -1313,7 +1313,17 @@ impl<'a, 'b, 'tcx> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b, 'tcx> {
13131313
_ => {
13141314
let orig_macro_rules_scope = self.parent_scope.macro_rules;
13151315
self.build_reduced_graph_for_item(item);
1316-
visit::walk_item(self, item);
1316+
match item.kind {
1317+
ItemKind::Mod(..) => {
1318+
// Visit attributes after items for backward compatibility.
1319+
// This way they can use `macro_rules` defined later.
1320+
self.visit_vis(&item.vis);
1321+
self.visit_ident(item.ident);
1322+
item.kind.walk(item, AssocCtxt::Trait, self);
1323+
visit::walk_list!(self, visit_attribute, &item.attrs);
1324+
}
1325+
_ => visit::walk_item(self, item),
1326+
}
13171327
match item.kind {
13181328
ItemKind::Mod(..) if self.contains_macro_use(&item.attrs) => {
13191329
self.parent_scope.macro_rules
@@ -1514,7 +1524,10 @@ impl<'a, 'b, 'tcx> Visitor<'b> for BuildReducedGraphVisitor<'a, 'b, 'tcx> {
15141524
if krate.is_placeholder {
15151525
self.visit_invoc_in_module(krate.id);
15161526
} else {
1517-
visit::walk_crate(self, krate);
1527+
// Visit attributes after items for backward compatibility.
1528+
// This way they can use `macro_rules` defined later.
1529+
visit::walk_list!(self, visit_item, &krate.items);
1530+
visit::walk_list!(self, visit_attribute, &krate.attrs);
15181531
self.contains_macro_use(&krate.attrs);
15191532
}
15201533
}

‎compiler/rustc_resolve/src/macros.rs

+71-9
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::errors::CannotDetermineMacroResolution;
55
use crate::errors::{self, AddAsNonDerive, CannotFindIdentInThisScope};
66
use crate::errors::{MacroExpectedFound, RemoveSurroundingDerive};
77
use crate::Namespace::*;
8-
use crate::{BindingKey, BuiltinMacroState, Determinacy, MacroData, Used};
8+
use crate::{BindingKey, BuiltinMacroState, Determinacy, MacroData, NameBindingKind, Used};
99
use crate::{DeriveData, Finalize, ParentScope, ResolutionError, Resolver, ScopeSet};
1010
use crate::{ModuleKind, ModuleOrUniformRoot, NameBinding, PathResult, Segment, ToNameBinding};
1111
use rustc_ast::expand::StrippedCfgItem;
@@ -18,15 +18,18 @@ use rustc_errors::{Applicability, StashKey};
1818
use rustc_expand::base::{Annotatable, DeriveResolution, Indeterminate, ResolverExpand};
1919
use rustc_expand::base::{SyntaxExtension, SyntaxExtensionKind};
2020
use rustc_expand::compile_declarative_macro;
21-
use rustc_expand::expand::{AstFragment, Invocation, InvocationKind, SupportsMacroExpansion};
21+
use rustc_expand::expand::{
22+
AstFragment, AstFragmentKind, Invocation, InvocationKind, SupportsMacroExpansion,
23+
};
2224
use rustc_hir::def::{self, DefKind, Namespace, NonMacroAttrKind};
2325
use rustc_hir::def_id::{CrateNum, DefId, LocalDefId};
2426
use rustc_middle::middle::stability;
2527
use rustc_middle::ty::RegisteredTools;
2628
use rustc_middle::ty::{TyCtxt, Visibility};
27-
use rustc_session::lint::builtin::UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES;
28-
use rustc_session::lint::builtin::{LEGACY_DERIVE_HELPERS, SOFT_UNSTABLE};
29-
use rustc_session::lint::builtin::{UNUSED_MACROS, UNUSED_MACRO_RULES};
29+
use rustc_session::lint::builtin::{
30+
LEGACY_DERIVE_HELPERS, OUT_OF_SCOPE_MACRO_CALLS, SOFT_UNSTABLE,
31+
UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES, UNUSED_MACROS, UNUSED_MACRO_RULES,
32+
};
3033
use rustc_session::lint::BuiltinLintDiag;
3134
use rustc_session::parse::feature_err;
3235
use rustc_span::edit_distance::edit_distance;
@@ -288,6 +291,16 @@ impl<'a, 'tcx> ResolverExpand for Resolver<'a, 'tcx> {
288291
let parent_scope = &ParentScope { derives, ..parent_scope };
289292
let supports_macro_expansion = invoc.fragment_kind.supports_macro_expansion();
290293
let node_id = invoc.expansion_data.lint_node_id;
294+
// This is a heuristic, but it's good enough for the lint.
295+
let looks_like_invoc_in_mod_inert_attr = self
296+
.invocation_parents
297+
.get(&invoc_id)
298+
.or_else(|| self.invocation_parents.get(&eager_expansion_root))
299+
.map(|&(mod_def_id, _)| mod_def_id)
300+
.filter(|&mod_def_id| {
301+
invoc.fragment_kind == AstFragmentKind::Expr
302+
&& self.tcx.def_kind(mod_def_id) == DefKind::Mod
303+
});
291304
let (ext, res) = self.smart_resolve_macro_path(
292305
path,
293306
kind,
@@ -298,6 +311,7 @@ impl<'a, 'tcx> ResolverExpand for Resolver<'a, 'tcx> {
298311
force,
299312
soft_custom_inner_attributes_gate(path, invoc),
300313
deleg_impl,
314+
looks_like_invoc_in_mod_inert_attr,
301315
)?;
302316

303317
let span = invoc.span();
@@ -520,6 +534,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
520534
force: bool,
521535
soft_custom_inner_attributes_gate: bool,
522536
deleg_impl: Option<LocalDefId>,
537+
invoc_in_mod_inert_attr: Option<LocalDefId>,
523538
) -> Result<(Lrc<SyntaxExtension>, Res), Indeterminate> {
524539
let (ext, res) = match self.resolve_macro_or_delegation_path(
525540
path,
@@ -528,6 +543,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
528543
true,
529544
force,
530545
deleg_impl,
546+
invoc_in_mod_inert_attr.map(|def_id| (def_id, node_id)),
531547
) {
532548
Ok((Some(ext), res)) => (ext, res),
533549
Ok((None, res)) => (self.dummy_ext(kind), res),
@@ -682,20 +698,21 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
682698
trace: bool,
683699
force: bool,
684700
) -> Result<(Option<Lrc<SyntaxExtension>>, Res), Determinacy> {
685-
self.resolve_macro_or_delegation_path(path, kind, parent_scope, trace, force, None)
701+
self.resolve_macro_or_delegation_path(path, kind, parent_scope, trace, force, None, None)
686702
}
687703

688704
fn resolve_macro_or_delegation_path(
689705
&mut self,
690-
path: &ast::Path,
706+
ast_path: &ast::Path,
691707
kind: Option<MacroKind>,
692708
parent_scope: &ParentScope<'a>,
693709
trace: bool,
694710
force: bool,
695711
deleg_impl: Option<LocalDefId>,
712+
invoc_in_mod_inert_attr: Option<(LocalDefId, NodeId)>,
696713
) -> Result<(Option<Lrc<SyntaxExtension>>, Res), Determinacy> {
697-
let path_span = path.span;
698-
let mut path = Segment::from_path(path);
714+
let path_span = ast_path.span;
715+
let mut path = Segment::from_path(ast_path);
699716

700717
// Possibly apply the macro helper hack
701718
if deleg_impl.is_none()
@@ -761,6 +778,12 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
761778

762779
let res = binding.map(|binding| binding.res());
763780
self.prohibit_imported_non_macro_attrs(binding.ok(), res.ok(), path_span);
781+
self.report_out_of_scope_macro_calls(
782+
ast_path,
783+
parent_scope,
784+
invoc_in_mod_inert_attr,
785+
binding.ok(),
786+
);
764787
res
765788
};
766789

@@ -1013,6 +1036,45 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
10131036
}
10141037
}
10151038

1039+
fn report_out_of_scope_macro_calls(
1040+
&mut self,
1041+
path: &ast::Path,
1042+
parent_scope: &ParentScope<'a>,
1043+
invoc_in_mod_inert_attr: Option<(LocalDefId, NodeId)>,
1044+
binding: Option<NameBinding<'a>>,
1045+
) {
1046+
if let Some((mod_def_id, node_id)) = invoc_in_mod_inert_attr
1047+
&& let Some(binding) = binding
1048+
// This is a `macro_rules` itself, not some import.
1049+
&& let NameBindingKind::Res(res) = binding.kind
1050+
&& let Res::Def(DefKind::Macro(MacroKind::Bang), def_id) = res
1051+
// And the `macro_rules` is defined inside the attribute's module,
1052+
// so it cannot be in scope unless imported.
1053+
&& self.tcx.is_descendant_of(def_id, mod_def_id.to_def_id())
1054+
{
1055+
// Try to resolve our ident ignoring `macro_rules` scopes.
1056+
// If such resolution is successful and gives the same result
1057+
// (e.g. if the macro is re-imported), then silence the lint.
1058+
let no_macro_rules = self.arenas.alloc_macro_rules_scope(MacroRulesScope::Empty);
1059+
let fallback_binding = self.early_resolve_ident_in_lexical_scope(
1060+
path.segments[0].ident,
1061+
ScopeSet::Macro(MacroKind::Bang),
1062+
&ParentScope { macro_rules: no_macro_rules, ..*parent_scope },
1063+
None,
1064+
false,
1065+
None,
1066+
);
1067+
if fallback_binding.ok().and_then(|b| b.res().opt_def_id()) != Some(def_id) {
1068+
self.tcx.sess.psess.buffer_lint(
1069+
OUT_OF_SCOPE_MACRO_CALLS,
1070+
path.span,
1071+
node_id,
1072+
BuiltinLintDiag::OutOfScopeMacroCalls { path: pprust::path_to_string(path) },
1073+
);
1074+
}
1075+
}
1076+
}
1077+
10161078
pub(crate) fn check_reserved_macro_name(&mut self, ident: Ident, res: Res) {
10171079
// Reserve some names that are not quite covered by the general check
10181080
// performed on `Resolver::builtin_attrs`.

‎tests/ui/attributes/key-value-expansion-scope-pass.rs

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Imports suppress the `out_of_scope_macro_calls` lint.
2+
3+
//@ check-pass
4+
//@ edition:2018
5+
6+
#![doc = in_root!()]
7+
8+
macro_rules! in_root { () => { "" } }
9+
use in_root;
10+
11+
mod macros_stay {
12+
#![doc = in_mod!()]
13+
14+
macro_rules! in_mod { () => { "" } }
15+
use in_mod;
16+
}
17+
18+
fn main() {}

‎tests/ui/attributes/key-value-expansion-scope.rs

+14-5
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1-
#![doc = in_root!()] //~ ERROR cannot find macro `in_root` in this scope
1+
#![doc = in_root!()] //~ WARN cannot find macro `in_root` in this scope
2+
//~| WARN this was previously accepted by the compiler
23
#![doc = in_mod!()] //~ ERROR cannot find macro `in_mod` in this scope
3-
#![doc = in_mod_escape!()] //~ ERROR cannot find macro `in_mod_escape` in this scope
4+
#![doc = in_mod_escape!()] //~ WARN cannot find macro `in_mod_escape` in this scope
5+
//~| WARN this was previously accepted by the compiler
46
#![doc = in_block!()] //~ ERROR cannot find macro `in_block` in this scope
57

68
#[doc = in_root!()] //~ ERROR cannot find macro `in_root` in this scope
@@ -16,8 +18,11 @@ fn before() {
1618

1719
macro_rules! in_root { () => { "" } }
1820

21+
#[doc = in_mod!()] //~ WARN cannot find macro `in_mod` in this scope
22+
//~| WARN this was previously accepted by the compiler
1923
mod macros_stay {
20-
#![doc = in_mod!()] //~ ERROR cannot find macro `in_mod` in this scope
24+
#![doc = in_mod!()] //~ WARN cannot find macro `in_mod` in this scope
25+
//~| WARN this was previously accepted by the compiler
2126

2227
macro_rules! in_mod { () => { "" } }
2328

@@ -28,8 +33,11 @@ mod macros_stay {
2833
}
2934

3035
#[macro_use]
36+
#[doc = in_mod_escape!()] //~ WARN cannot find macro `in_mod_escape` in this scope
37+
//~| WARN this was previously accepted by the compiler
3138
mod macros_escape {
32-
#![doc = in_mod_escape!()] //~ ERROR cannot find macro `in_mod_escape` in this scope
39+
#![doc = in_mod_escape!()] //~ WARN cannot find macro `in_mod_escape` in this scope
40+
//~| WARN this was previously accepted by the compiler
3341

3442
macro_rules! in_mod_escape { () => { "" } }
3543

@@ -39,8 +47,9 @@ mod macros_escape {
3947
}
4048
}
4149

50+
#[doc = in_block!()] //~ ERROR cannot find macro `in_block` in this scope
4251
fn block() {
43-
#![doc = in_block!()] //~ ERROR cannot find macro `in_block` in this scope
52+
#![doc = in_block!()] //~ ERROR cannot find macro `in_block` in this scope
4453

4554
macro_rules! in_block { () => { "" } }
4655

0 commit comments

Comments
 (0)