Auto merge of #87296 - Aaron1011:inert-warn, r=petrochenkov

Warn on inert attributes used on bang macro invocation

These attributes are currently discarded.
This may change in the future (see #63221), but for now,
placing inert attributes on a macro invocation does nothing,
so we should warn users about it.

Technically, it's possible for there to be attribute macro
on the same macro invocation (or at a higher scope), which
inspects the inert attribute. For example:

```rust
#[look_for_inline_attr]
#[inline]
my_macro!()

#[look_for_nested_inline]
mod foo { #[inline] my_macro!() }
```

However, this would be a very strange thing to do.
Anyone running into this can manually suppress the warning.
This commit is contained in:
bors 2021-07-24 13:19:17 +00:00
commit 18840b0719
7 changed files with 115 additions and 24 deletions

View File

@ -12,7 +12,7 @@ use rustc_ast::ptr::P;
use rustc_ast::token; use rustc_ast::token;
use rustc_ast::tokenstream::TokenStream; use rustc_ast::tokenstream::TokenStream;
use rustc_ast::visit::{self, AssocCtxt, Visitor}; use rustc_ast::visit::{self, AssocCtxt, Visitor};
use rustc_ast::{AstLike, Block, Inline, ItemKind, Local, MacArgs}; use rustc_ast::{AstLike, Block, Inline, ItemKind, Local, MacArgs, MacCall};
use rustc_ast::{MacCallStmt, MacStmtStyle, MetaItemKind, ModKind, NestedMetaItem}; use rustc_ast::{MacCallStmt, MacStmtStyle, MetaItemKind, ModKind, NestedMetaItem};
use rustc_ast::{NodeId, PatKind, Path, StmtKind, Unsafe}; use rustc_ast::{NodeId, PatKind, Path, StmtKind, Unsafe};
use rustc_ast_pretty::pprust; use rustc_ast_pretty::pprust;
@ -26,7 +26,7 @@ use rustc_parse::parser::{
AttemptLocalParseRecovery, ForceCollect, Parser, RecoverColon, RecoverComma, AttemptLocalParseRecovery, ForceCollect, Parser, RecoverColon, RecoverComma,
}; };
use rustc_parse::validate_attr; use rustc_parse::validate_attr;
use rustc_session::lint::builtin::UNUSED_DOC_COMMENTS; use rustc_session::lint::builtin::{UNUSED_ATTRIBUTES, UNUSED_DOC_COMMENTS};
use rustc_session::lint::BuiltinLintDiagnostics; use rustc_session::lint::BuiltinLintDiagnostics;
use rustc_session::parse::{feature_err, ParseSess}; use rustc_session::parse::{feature_err, ParseSess};
use rustc_session::Limit; use rustc_session::Limit;
@ -1070,7 +1070,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
// Detect use of feature-gated or invalid attributes on macro invocations // Detect use of feature-gated or invalid attributes on macro invocations
// since they will not be detected after macro expansion. // since they will not be detected after macro expansion.
fn check_attributes(&mut self, attrs: &[ast::Attribute]) { fn check_attributes(&mut self, attrs: &[ast::Attribute], call: &MacCall) {
let features = self.cx.ecfg.features.unwrap(); let features = self.cx.ecfg.features.unwrap();
let mut attrs = attrs.iter().peekable(); let mut attrs = attrs.iter().peekable();
let mut span: Option<Span> = None; let mut span: Option<Span> = None;
@ -1085,14 +1085,31 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
continue; continue;
} }
if attr.doc_str().is_some() { if attr.is_doc_comment() {
self.cx.sess.parse_sess.buffer_lint_with_diagnostic( self.cx.sess.parse_sess.buffer_lint_with_diagnostic(
&UNUSED_DOC_COMMENTS, &UNUSED_DOC_COMMENTS,
current_span, current_span,
ast::CRATE_NODE_ID, self.cx.current_expansion.lint_node_id,
"unused doc comment", "unused doc comment",
BuiltinLintDiagnostics::UnusedDocComment(attr.span), BuiltinLintDiagnostics::UnusedDocComment(attr.span),
); );
} else if rustc_attr::is_builtin_attr(attr) {
let attr_name = attr.ident().unwrap().name;
// `#[cfg]` and `#[cfg_attr]` are special - they are
// eagerly evaluated.
if attr_name != sym::cfg && attr_name != sym::cfg_attr {
self.cx.sess.parse_sess.buffer_lint_with_diagnostic(
&UNUSED_ATTRIBUTES,
attr.span,
self.cx.current_expansion.lint_node_id,
&format!("unused attribute `{}`", attr_name),
BuiltinLintDiagnostics::UnusedBuiltinAttribute {
attr_name,
macro_name: pprust::path_to_string(&call.path),
invoc_span: call.path.span,
},
);
}
} }
} }
} }
@ -1152,7 +1169,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
} }
if let ast::ExprKind::MacCall(mac) = expr.kind { if let ast::ExprKind::MacCall(mac) = expr.kind {
self.check_attributes(&expr.attrs); self.check_attributes(&expr.attrs, &mac);
self.collect_bang(mac, expr.span, AstFragmentKind::Expr).make_expr().into_inner() self.collect_bang(mac, expr.span, AstFragmentKind::Expr).make_expr().into_inner()
} else { } else {
assign_id!(self, &mut expr.id, || { assign_id!(self, &mut expr.id, || {
@ -1253,7 +1270,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
} }
if let ast::ExprKind::MacCall(mac) = expr.kind { if let ast::ExprKind::MacCall(mac) = expr.kind {
self.check_attributes(&expr.attrs); self.check_attributes(&expr.attrs, &mac);
self.collect_bang(mac, expr.span, AstFragmentKind::OptExpr) self.collect_bang(mac, expr.span, AstFragmentKind::OptExpr)
.make_opt_expr() .make_opt_expr()
.map(|expr| expr.into_inner()) .map(|expr| expr.into_inner())
@ -1296,7 +1313,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
if let StmtKind::MacCall(mac) = stmt.kind { if let StmtKind::MacCall(mac) = stmt.kind {
let MacCallStmt { mac, style, attrs, tokens: _ } = mac.into_inner(); let MacCallStmt { mac, style, attrs, tokens: _ } = mac.into_inner();
self.check_attributes(&attrs); self.check_attributes(&attrs, &mac);
let mut placeholder = let mut placeholder =
self.collect_bang(mac, stmt.span, AstFragmentKind::Stmts).make_stmts(); self.collect_bang(mac, stmt.span, AstFragmentKind::Stmts).make_stmts();
@ -1344,9 +1361,9 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
let span = item.span; let span = item.span;
match item.kind { match item.kind {
ast::ItemKind::MacCall(..) => { ast::ItemKind::MacCall(ref mac) => {
self.check_attributes(&attrs, &mac);
item.attrs = attrs; item.attrs = attrs;
self.check_attributes(&item.attrs);
item.and_then(|item| match item.kind { item.and_then(|item| match item.kind {
ItemKind::MacCall(mac) => { ItemKind::MacCall(mac) => {
self.collect_bang(mac, span, AstFragmentKind::Items).make_items() self.collect_bang(mac, span, AstFragmentKind::Items).make_items()
@ -1455,8 +1472,8 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
} }
match item.kind { match item.kind {
ast::AssocItemKind::MacCall(..) => { ast::AssocItemKind::MacCall(ref mac) => {
self.check_attributes(&item.attrs); self.check_attributes(&item.attrs, &mac);
item.and_then(|item| match item.kind { item.and_then(|item| match item.kind {
ast::AssocItemKind::MacCall(mac) => self ast::AssocItemKind::MacCall(mac) => self
.collect_bang(mac, item.span, AstFragmentKind::TraitItems) .collect_bang(mac, item.span, AstFragmentKind::TraitItems)
@ -1480,8 +1497,8 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
} }
match item.kind { match item.kind {
ast::AssocItemKind::MacCall(..) => { ast::AssocItemKind::MacCall(ref mac) => {
self.check_attributes(&item.attrs); self.check_attributes(&item.attrs, &mac);
item.and_then(|item| match item.kind { item.and_then(|item| match item.kind {
ast::AssocItemKind::MacCall(mac) => self ast::AssocItemKind::MacCall(mac) => self
.collect_bang(mac, item.span, AstFragmentKind::ImplItems) .collect_bang(mac, item.span, AstFragmentKind::ImplItems)
@ -1526,8 +1543,8 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
} }
match foreign_item.kind { match foreign_item.kind {
ast::ForeignItemKind::MacCall(..) => { ast::ForeignItemKind::MacCall(ref mac) => {
self.check_attributes(&foreign_item.attrs); self.check_attributes(&foreign_item.attrs, &mac);
foreign_item.and_then(|item| match item.kind { foreign_item.and_then(|item| match item.kind {
ast::ForeignItemKind::MacCall(mac) => self ast::ForeignItemKind::MacCall(mac) => self
.collect_bang(mac, item.span, AstFragmentKind::ForeignItems) .collect_bang(mac, item.span, AstFragmentKind::ForeignItems)

View File

@ -734,6 +734,16 @@ pub trait LintContext: Sized {
Applicability::MachineApplicable, Applicability::MachineApplicable,
); );
} }
BuiltinLintDiagnostics::UnusedBuiltinAttribute {
attr_name,
macro_name,
invoc_span
} => {
db.span_note(
invoc_span,
&format!("the built-in attribute `{attr_name}` will be ignored, since it's applied to the macro invocation `{macro_name}`")
);
}
} }
// Rewrap `db`, and pass control to the user. // Rewrap `db`, and pass control to the user.
decorate(LintDiagnosticBuilder::new(db)); decorate(LintDiagnosticBuilder::new(db));

View File

@ -32,6 +32,7 @@
#![feature(box_syntax)] #![feature(box_syntax)]
#![feature(box_patterns)] #![feature(box_patterns)]
#![feature(crate_visibility_modifier)] #![feature(crate_visibility_modifier)]
#![feature(format_args_capture)]
#![feature(iter_order_by)] #![feature(iter_order_by)]
#![feature(iter_zip)] #![feature(iter_zip)]
#![feature(never_type)] #![feature(never_type)]

View File

@ -296,6 +296,7 @@ pub enum BuiltinLintDiagnostics {
DeprecatedMacro(Option<Symbol>, Span), DeprecatedMacro(Option<Symbol>, Span),
MissingAbi(Span, Abi), MissingAbi(Span, Abi),
UnusedDocComment(Span), UnusedDocComment(Span),
UnusedBuiltinAttribute { attr_name: Symbol, macro_name: String, invoc_span: Span },
PatternsInFnsWithoutBody(Span, Ident), PatternsInFnsWithoutBody(Span, Ident),
LegacyDeriveHelpers(Span), LegacyDeriveHelpers(Span),
ExternDepSpec(String, ExternDepSpec), ExternDepSpec(String, ExternDepSpec),

View File

@ -0,0 +1,20 @@
// check-pass
#![warn(unused)]
macro_rules! foo {
() => {}
}
fn main() {
#[inline] foo!(); //~ WARN unused attribute `inline`
// This does nothing, since `#[allow(warnings)]` is itself
// an inert attribute on a macro call
#[allow(warnings)] #[inline] foo!(); //~ WARN unused attribute `allow`
//~^ WARN unused attribute `inline`
// This does work, since the attribute is on a parent
// of the macro invocation.
#[allow(warnings)] { #[inline] foo!(); }
}

View File

@ -0,0 +1,44 @@
warning: unused attribute `inline`
--> $DIR/inert-attr-macro.rs:10:5
|
LL | #[inline] foo!();
| ^^^^^^^^^
|
note: the lint level is defined here
--> $DIR/inert-attr-macro.rs:3:9
|
LL | #![warn(unused)]
| ^^^^^^
= note: `#[warn(unused_attributes)]` implied by `#[warn(unused)]`
note: the built-in attribute `inline` will be ignored, since it's applied to the macro invocation `foo`
--> $DIR/inert-attr-macro.rs:10:15
|
LL | #[inline] foo!();
| ^^^
warning: unused attribute `allow`
--> $DIR/inert-attr-macro.rs:14:5
|
LL | #[allow(warnings)] #[inline] foo!();
| ^^^^^^^^^^^^^^^^^^
|
note: the built-in attribute `allow` will be ignored, since it's applied to the macro invocation `foo`
--> $DIR/inert-attr-macro.rs:14:34
|
LL | #[allow(warnings)] #[inline] foo!();
| ^^^
warning: unused attribute `inline`
--> $DIR/inert-attr-macro.rs:14:24
|
LL | #[allow(warnings)] #[inline] foo!();
| ^^^^^^^^^
|
note: the built-in attribute `inline` will be ignored, since it's applied to the macro invocation `foo`
--> $DIR/inert-attr-macro.rs:14:34
|
LL | #[allow(warnings)] #[inline] foo!();
| ^^^
warning: 3 warnings emitted

View File

@ -73,8 +73,7 @@ mod enum_inline {
// general; this test is relying on that.) // general; this test is relying on that.)
two_fifty_six_variant_enum!(Visible2, N8); two_fifty_six_variant_enum!(Visible2, N8);
#[repr(no_niche)] two_fifty_six_variant_enum!(#[repr(no_niche)] Cloaked2, N8);
two_fifty_six_variant_enum!(Cloaked2, N8);
} }
mod enum_param { mod enum_param {
@ -96,8 +95,7 @@ mod enum_param {
// here as above (assuming `T` is instantiated with `NonZeroU8`). // here as above (assuming `T` is instantiated with `NonZeroU8`).
two_fifty_six_variant_enum!(Visible2<T>); two_fifty_six_variant_enum!(Visible2<T>);
#[repr(no_niche)] two_fifty_six_variant_enum!(#[repr(no_niche)] Cloaked2<T>);
two_fifty_six_variant_enum!(Cloaked2<T>);
} }
fn main() { fn main() {
@ -157,8 +155,8 @@ fn main() {
} }
macro two_fifty_six_variant_enum { macro two_fifty_six_variant_enum {
($name:ident<$param:ident>) => { ($(#[$attr:meta])* $name:ident<$param:ident>) => {
#[derive(Debug)] #[derive(Debug)] $(#[$attr])*
pub enum $name<$param> { pub enum $name<$param> {
_V00($param, u16), _V01(u16, $param), _V02($param, u16), _V03(u16, $param), _V00($param, u16), _V01(u16, $param), _V02($param, u16), _V03(u16, $param),
_V04($param, u16), _V05(u16, $param), _V06($param, u16), _V07(u16, $param), _V04($param, u16), _V05(u16, $param), _V06($param, u16), _V07(u16, $param),
@ -242,8 +240,8 @@ macro two_fifty_six_variant_enum {
} }
}, },
($name:ident, $param:ty) => { ($(#[$attr:meta])* $name:ident, $param:ty) => {
#[derive(Debug)] #[derive(Debug)] $(#[$attr])*
pub enum $name { pub enum $name {
_V00($param, u16), _V01(u16, $param), _V02($param, u16), _V03(u16, $param), _V00($param, u16), _V01(u16, $param), _V02($param, u16), _V03(u16, $param),
_V04($param, u16), _V05(u16, $param), _V06($param, u16), _V07(u16, $param), _V04($param, u16), _V05(u16, $param), _V06($param, u16), _V07(u16, $param),