Display an extra note for trailing semicolon lint with trailing macro

Currently, we parse macros at the end of a block
(e.g. `fn foo() { my_macro!() }`) as expressions, rather than
statements. This means that a macro invoked in this position
cannot expand to items or semicolon-terminated expressions.

In the future, we might want to start parsing these kinds of macros
as statements. This would make expansion more 'token-based'
(i.e. macro expansion behaves (almost) as if you just textually
replaced the macro invocation with its output). However,
this is a breaking change (see PR #78991), so it will require
further discussion.

Since the current behavior will not be changing any time soon,
we need to address the interaction with the
`SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` lint. Since we are parsing
the result of macro expansion as an expression, we will emit a lint
if there's a trailing semicolon in the macro output. However, this
results in a somewhat confusing message for users, since it visually
looks like there should be no problem with having a semicolon
at the end of a block
(e.g. `fn foo() { my_macro!() }` => `fn foo() { produced_expr; }`)

To help reduce confusion, this commit adds a note explaining
that the macro is being interpreted as an expression. Additionally,
we suggest adding a semicolon after the macro *invocation* - this
will cause us to parse the macro call as a statement. We do *not*
use a structured suggestion for this, since the user may actually
want to remove the semicolon from the macro definition (allowing
the block to evaluate to the expression produced by the macro).
This commit is contained in:
Aaron Hill 2021-07-22 11:24:42 -05:00
parent 18840b0719
commit 0df5ac8269
No known key found for this signature in database
GPG Key ID: B4087E510E98B164
6 changed files with 41 additions and 5 deletions

View File

@ -928,6 +928,7 @@ pub struct ExpansionData {
pub prior_type_ascription: Option<(Span, bool)>,
/// Some parent node that is close to this macro call
pub lint_node_id: NodeId,
pub is_trailing_mac: bool,
}
type OnExternModLoaded<'a> =
@ -974,6 +975,7 @@ impl<'a> ExtCtxt<'a> {
dir_ownership: DirOwnership::Owned { relative: None },
prior_type_ascription: None,
lint_node_id: ast::CRATE_NODE_ID,
is_trailing_mac: false,
},
force_mode: false,
expansions: FxHashMap::default(),

View File

@ -1328,14 +1328,30 @@ impl<'a, 'b> MutVisitor for InvocationCollector<'a, 'b> {
return placeholder;
}
// The only way that we can end up with a `MacCall` expression statement,
// (as opposed to a `StmtKind::MacCall`) is if we have a macro as the
// traiing expression in a block (e.g. `fn foo() { my_macro!() }`).
// Record this information, so that we can report a more specific
// `SEMICOLON_IN_EXPRESSIONS_FROM_MACROS` lint if needed.
// See #78991 for an investigation of treating macros in this position
// as statements, rather than expressions, during parsing.
if let StmtKind::Expr(expr) = &stmt.kind {
if matches!(**expr, ast::Expr { kind: ast::ExprKind::MacCall(..), .. }) {
self.cx.current_expansion.is_trailing_mac = true;
}
}
// The placeholder expander gives ids to statements, so we avoid folding the id here.
// We don't use `assign_id!` - it will be called when we visit statement's contents
// (e.g. an expression, item, or local)
let ast::Stmt { id, kind, span } = stmt;
noop_flat_map_stmt_kind(kind, self)
let res = noop_flat_map_stmt_kind(kind, self)
.into_iter()
.map(|kind| ast::Stmt { id, kind, span })
.collect()
.collect();
self.cx.current_expansion.is_trailing_mac = false;
res
}
fn visit_block(&mut self, block: &mut P<Block>) {

View File

@ -43,6 +43,7 @@ crate struct ParserAnyMacro<'a> {
/// The ident of the macro we're parsing
macro_ident: Ident,
lint_node_id: NodeId,
is_trailing_mac: bool,
arm_span: Span,
}
@ -116,8 +117,14 @@ fn emit_frag_parse_err(
impl<'a> ParserAnyMacro<'a> {
crate fn make(mut self: Box<ParserAnyMacro<'a>>, kind: AstFragmentKind) -> AstFragment {
let ParserAnyMacro { site_span, macro_ident, ref mut parser, lint_node_id, arm_span } =
*self;
let ParserAnyMacro {
site_span,
macro_ident,
ref mut parser,
lint_node_id,
arm_span,
is_trailing_mac,
} = *self;
let snapshot = &mut parser.clone();
let fragment = match parse_ast_fragment(parser, kind) {
Ok(f) => f,
@ -131,11 +138,12 @@ impl<'a> ParserAnyMacro<'a> {
// `macro_rules! m { () => { panic!(); } }` isn't parsed by `.parse_expr()`,
// but `m!()` is allowed in expression positions (cf. issue #34706).
if kind == AstFragmentKind::Expr && parser.token == token::Semi {
parser.sess.buffer_lint(
parser.sess.buffer_lint_with_diagnostic(
SEMICOLON_IN_EXPRESSIONS_FROM_MACROS,
parser.token.span,
lint_node_id,
"trailing semicolon in macro used in expression position",
BuiltinLintDiagnostics::TrailingMacro(is_trailing_mac, macro_ident),
);
parser.bump();
}
@ -301,6 +309,7 @@ fn generic_extension<'cx>(
site_span: sp,
macro_ident: name,
lint_node_id: cx.current_expansion.lint_node_id,
is_trailing_mac: cx.current_expansion.is_trailing_mac,
arm_span,
});
}

View File

@ -744,6 +744,12 @@ pub trait LintContext: Sized {
&format!("the built-in attribute `{attr_name}` will be ignored, since it's applied to the macro invocation `{macro_name}`")
);
}
BuiltinLintDiagnostics::TrailingMacro(is_trailing, name) => {
if is_trailing {
db.note("macro invocations at the end of a block are treated as expressions");
db.note(&format!("to ignore the value produced by the macro, add a semicolon after the invocation of `{name}`"));
}
}
}
// Rewrap `db`, and pass control to the user.
decorate(LintDiagnosticBuilder::new(db));

View File

@ -303,6 +303,7 @@ pub enum BuiltinLintDiagnostics {
ProcMacroBackCompat(String),
OrPatternsBackCompat(Span, String),
ReservedPrefix(Span),
TrailingMacro(bool, Ident),
}
/// Lints that are buffered up early on in the `Session` before the

View File

@ -14,6 +14,8 @@ LL | #![warn(semicolon_in_expressions_from_macros)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
= 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 #79813 <https://github.com/rust-lang/rust/issues/79813>
= note: macro invocations at the end of a block are treated as expressions
= note: to ignore the value produced by the macro, add a semicolon after the invocation of `foo`
= note: this warning originates in the macro `foo` (in Nightly builds, run with -Z macro-backtrace for more info)
warning: trailing semicolon in macro used in expression position