Skip to content

Commit

Permalink
Warn on inert attributes used on bang macro invocation
Browse files Browse the repository at this point in the history
These attributes are currently discarded.
This may change in the future (see rust-lang#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.
  • Loading branch information
Aaron1011 committed Jul 19, 2021
1 parent 8df945c commit 070df9e
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 16 deletions.
49 changes: 33 additions & 16 deletions compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_ast::ptr::P;
use rustc_ast::token;
use rustc_ast::tokenstream::TokenStream;
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::{NodeId, PatKind, Path, StmtKind, Unsafe};
use rustc_ast_pretty::pprust;
Expand All @@ -26,7 +26,7 @@ use rustc_parse::parser::{
AttemptLocalParseRecovery, ForceCollect, Parser, RecoverColon, RecoverComma,
};
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::parse::{feature_err, ParseSess};
use rustc_session::Limit;
Expand Down Expand Up @@ -1070,7 +1070,7 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {

// Detect use of feature-gated or invalid attributes on macro invocations
// 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 mut attrs = attrs.iter().peekable();
let mut span: Option<Span> = None;
Expand All @@ -1085,14 +1085,31 @@ impl<'a, 'b> InvocationCollector<'a, 'b> {
continue;
}

if attr.doc_str().is_some() {
if attr.is_doc_comment() {
self.cx.sess.parse_sess.buffer_lint_with_diagnostic(
&UNUSED_DOC_COMMENTS,
current_span,
ast::CRATE_NODE_ID,
self.cx.current_expansion.lint_node_id,
"unused doc comment",
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,
},
);
}
}
}
}
Expand Down Expand Up @@ -1152,7 +1169,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
}

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()
} else {
assign_id!(self, &mut expr.id, || {
Expand Down Expand Up @@ -1253,7 +1270,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
}

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)
.make_opt_expr()
.map(|expr| expr.into_inner())
Expand Down Expand Up @@ -1296,7 +1313,7 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {

if let StmtKind::MacCall(mac) = stmt.kind {
let MacCallStmt { mac, style, attrs, tokens: _ } = mac.into_inner();
self.check_attributes(&attrs);
self.check_attributes(&attrs, &mac);
let mut placeholder =
self.collect_bang(mac, stmt.span, AstFragmentKind::Stmts).make_stmts();

Expand Down Expand Up @@ -1344,9 +1361,9 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
let span = item.span;

match item.kind {
ast::ItemKind::MacCall(..) => {
ast::ItemKind::MacCall(ref mac) => {
self.check_attributes(&attrs, &mac);
item.attrs = attrs;
self.check_attributes(&item.attrs);
item.and_then(|item| match item.kind {
ItemKind::MacCall(mac) => {
self.collect_bang(mac, span, AstFragmentKind::Items).make_items()
Expand Down Expand Up @@ -1455,8 +1472,8 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
}

match item.kind {
ast::AssocItemKind::MacCall(..) => {
self.check_attributes(&item.attrs);
ast::AssocItemKind::MacCall(ref mac) => {
self.check_attributes(&item.attrs, &mac);
item.and_then(|item| match item.kind {
ast::AssocItemKind::MacCall(mac) => self
.collect_bang(mac, item.span, AstFragmentKind::TraitItems)
Expand All @@ -1480,8 +1497,8 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
}

match item.kind {
ast::AssocItemKind::MacCall(..) => {
self.check_attributes(&item.attrs);
ast::AssocItemKind::MacCall(ref mac) => {
self.check_attributes(&item.attrs, &mac);
item.and_then(|item| match item.kind {
ast::AssocItemKind::MacCall(mac) => self
.collect_bang(mac, item.span, AstFragmentKind::ImplItems)
Expand Down Expand Up @@ -1526,8 +1543,8 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
}

match foreign_item.kind {
ast::ForeignItemKind::MacCall(..) => {
self.check_attributes(&foreign_item.attrs);
ast::ForeignItemKind::MacCall(ref mac) => {
self.check_attributes(&foreign_item.attrs, &mac);
foreign_item.and_then(|item| match item.kind {
ast::ForeignItemKind::MacCall(mac) => self
.collect_bang(mac, item.span, AstFragmentKind::ForeignItems)
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_lint/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,16 @@ pub trait LintContext: Sized {
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.
decorate(LintDiagnosticBuilder::new(db));
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#![feature(box_syntax)]
#![feature(box_patterns)]
#![feature(crate_visibility_modifier)]
#![feature(format_args_capture)]
#![feature(iter_order_by)]
#![feature(iter_zip)]
#![feature(never_type)]
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_lint_defs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ pub enum BuiltinLintDiagnostics {
DeprecatedMacro(Option<Symbol>, Span),
MissingAbi(Span, Abi),
UnusedDocComment(Span),
UnusedBuiltinAttribute { attr_name: Symbol, macro_name: String, invoc_span: Span },
PatternsInFnsWithoutBody(Span, Ident),
LegacyDeriveHelpers(Span),
ExternDepSpec(String, ExternDepSpec),
Expand Down
20 changes: 20 additions & 0 deletions src/test/ui/lint/inert-attr-macro.rs
Original file line number Diff line number Diff line change
@@ -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!(); }
}
44 changes: 44 additions & 0 deletions src/test/ui/lint/inert-attr-macro.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 070df9e

Please sign in to comment.