From f56a4c0ebf942389498ff7bf4273b09d8238cfe6 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 18 Dec 2023 02:11:55 +0000 Subject: [PATCH 1/2] Deny braced macro invocations in let-else --- compiler/rustc_ast/src/util/classify.rs | 10 +++++-- tests/ui/parser/bad-let-else-statement.rs | 27 ++++++++++++++++++ tests/ui/parser/bad-let-else-statement.stderr | 28 ++++++++++++++++++- 3 files changed, 61 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_ast/src/util/classify.rs b/compiler/rustc_ast/src/util/classify.rs index 65036bcdc36fd..098b8f2d6d038 100644 --- a/compiler/rustc_ast/src/util/classify.rs +++ b/compiler/rustc_ast/src/util/classify.rs @@ -2,7 +2,7 @@ // Predicates on exprs and stmts that the pretty-printer and parser use -use crate::ast; +use crate::{ast, token::Delimiter}; /// Does this expression require a semicolon to be treated /// as a statement? The negation of this: 'can this expression @@ -59,8 +59,12 @@ pub fn expr_trailing_brace(mut expr: &ast::Expr) -> Option<&ast::Expr> { | While(..) | ConstBlock(_) => break Some(expr), - // FIXME: These can end in `}`, but changing these would break stable code. - InlineAsm(_) | OffsetOf(_, _) | MacCall(_) | IncludedBytes(_) | FormatArgs(_) => { + MacCall(mac) => { + break (mac.args.delim == Delimiter::Brace).then_some(expr); + } + + InlineAsm(_) | OffsetOf(_, _) | IncludedBytes(_) | FormatArgs(_) => { + // These should have been denied pre-expansion. break None; } diff --git a/tests/ui/parser/bad-let-else-statement.rs b/tests/ui/parser/bad-let-else-statement.rs index 7b927c89ba074..5866e1b751461 100644 --- a/tests/ui/parser/bad-let-else-statement.rs +++ b/tests/ui/parser/bad-let-else-statement.rs @@ -161,4 +161,31 @@ fn q() { }; } +fn r() { + use std::arch::asm; + + let ok = asm!(" nop ") else { return; }; + + let bad = asm! {" nop "} else { return; }; + //~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed +} + +fn s() { + macro_rules! a { + () => { {} } + } + + macro_rules! b { + (1) => { + let x = a!() else { return; }; + }; + (2) => { + let x = a! {} else { return; }; + //~^ ERROR right curly brace `}` before `else` in a `let...else` statement not allowed + }; + } + + b!(1); b!(2); +} + fn main() {} diff --git a/tests/ui/parser/bad-let-else-statement.stderr b/tests/ui/parser/bad-let-else-statement.stderr index 7cbda25e417f3..e636a2226e205 100644 --- a/tests/ui/parser/bad-let-else-statement.stderr +++ b/tests/ui/parser/bad-let-else-statement.stderr @@ -228,5 +228,31 @@ LL | x LL ~ }) else { | -error: aborting due to 17 previous errors +error: right curly brace `}` before `else` in a `let...else` statement not allowed + --> $DIR/bad-let-else-statement.rs:169:28 + | +LL | let bad = asm! {" nop "} else { return; }; + | ^ + | +help: wrap the expression in parentheses + | +LL | let bad = (asm! {" nop "}) else { return; }; + | + + + +error: right curly brace `}` before `else` in a `let...else` statement not allowed + --> $DIR/bad-let-else-statement.rs:183:25 + | +LL | let x = a! {} else { return; }; + | ^ +... +LL | b!(1); b!(2); + | ----- in this macro invocation + | + = note: this error originates in the macro `b` (in Nightly builds, run with -Z macro-backtrace for more info) +help: wrap the expression in parentheses + | +LL | let x = (a! {}) else { return; }; + | + + + +error: aborting due to 19 previous errors From fd023696658c62b82e0b0b67578cf0733e183a9e Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 18 Dec 2023 03:01:05 +0000 Subject: [PATCH 2/2] Suggest wrapping mac args in parens rather than the whole expression --- compiler/rustc_parse/messages.ftl | 2 + compiler/rustc_parse/src/errors.rs | 37 +++++++++++++------ compiler/rustc_parse/src/parser/expr.rs | 2 +- compiler/rustc_parse/src/parser/stmt.rs | 15 ++++++-- tests/ui/parser/bad-let-else-statement.stderr | 12 +++--- 5 files changed, 45 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_parse/messages.ftl b/compiler/rustc_parse/messages.ftl index d515e86a18296..e3ecd24b6a5f0 100644 --- a/compiler/rustc_parse/messages.ftl +++ b/compiler/rustc_parse/messages.ftl @@ -722,6 +722,8 @@ parse_sugg_turbofish_syntax = use `::<...>` instead of `<...>` to specify lifeti parse_sugg_wrap_expression_in_parentheses = wrap the expression in parentheses +parse_sugg_wrap_macro_in_parentheses = use parentheses instead of braces for this macro + parse_sugg_wrap_pattern_in_parens = wrap the pattern in parentheses parse_switch_mut_let_order = diff --git a/compiler/rustc_parse/src/errors.rs b/compiler/rustc_parse/src/errors.rs index 34b34a1bad6c9..8f57b2eb5aa97 100644 --- a/compiler/rustc_parse/src/errors.rs +++ b/compiler/rustc_parse/src/errors.rs @@ -722,19 +722,32 @@ pub(crate) struct LabeledLoopInBreak { #[primary_span] pub span: Span, #[subdiagnostic] - pub sub: WrapExpressionInParentheses, + pub sub: WrapInParentheses, } #[derive(Subdiagnostic)] -#[multipart_suggestion( - parse_sugg_wrap_expression_in_parentheses, - applicability = "machine-applicable" -)] -pub(crate) struct WrapExpressionInParentheses { - #[suggestion_part(code = "(")] - pub left: Span, - #[suggestion_part(code = ")")] - pub right: Span, + +pub(crate) enum WrapInParentheses { + #[multipart_suggestion( + parse_sugg_wrap_expression_in_parentheses, + applicability = "machine-applicable" + )] + Expression { + #[suggestion_part(code = "(")] + left: Span, + #[suggestion_part(code = ")")] + right: Span, + }, + #[multipart_suggestion( + parse_sugg_wrap_macro_in_parentheses, + applicability = "machine-applicable" + )] + MacroArgs { + #[suggestion_part(code = "(")] + left: Span, + #[suggestion_part(code = ")")] + right: Span, + }, } #[derive(Diagnostic)] @@ -936,7 +949,7 @@ pub(crate) struct InvalidExpressionInLetElse { pub span: Span, pub operator: &'static str, #[subdiagnostic] - pub sugg: WrapExpressionInParentheses, + pub sugg: WrapInParentheses, } #[derive(Diagnostic)] @@ -945,7 +958,7 @@ pub(crate) struct InvalidCurlyInLetElse { #[primary_span] pub span: Span, #[subdiagnostic] - pub sugg: WrapExpressionInParentheses, + pub sugg: WrapInParentheses, } #[derive(Diagnostic)] diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 8ca02452342b2..f858706805dbc 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1844,7 +1844,7 @@ impl<'a> Parser<'a> { let lexpr = self.parse_expr_labeled(label, true)?; self.dcx().emit_err(errors::LabeledLoopInBreak { span: lexpr.span, - sub: errors::WrapExpressionInParentheses { + sub: errors::WrapInParentheses::Expression { left: lexpr.span.shrink_to_lo(), right: lexpr.span.shrink_to_hi(), }, diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index 1a8bbf0a15722..1bae5b3224035 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -389,7 +389,7 @@ impl<'a> Parser<'a> { self.dcx().emit_err(errors::InvalidExpressionInLetElse { span: init.span, operator: op.node.as_str(), - sugg: errors::WrapExpressionInParentheses { + sugg: errors::WrapInParentheses::Expression { left: init.span.shrink_to_lo(), right: init.span.shrink_to_hi(), }, @@ -400,12 +400,19 @@ impl<'a> Parser<'a> { fn check_let_else_init_trailing_brace(&self, init: &ast::Expr) { if let Some(trailing) = classify::expr_trailing_brace(init) { - self.dcx().emit_err(errors::InvalidCurlyInLetElse { - span: trailing.span.with_lo(trailing.span.hi() - BytePos(1)), - sugg: errors::WrapExpressionInParentheses { + let sugg = match &trailing.kind { + ExprKind::MacCall(mac) => errors::WrapInParentheses::MacroArgs { + left: mac.args.dspan.open, + right: mac.args.dspan.close, + }, + _ => errors::WrapInParentheses::Expression { left: trailing.span.shrink_to_lo(), right: trailing.span.shrink_to_hi(), }, + }; + self.dcx().emit_err(errors::InvalidCurlyInLetElse { + span: trailing.span.with_lo(trailing.span.hi() - BytePos(1)), + sugg, }); } } diff --git a/tests/ui/parser/bad-let-else-statement.stderr b/tests/ui/parser/bad-let-else-statement.stderr index e636a2226e205..7201abbb58ca0 100644 --- a/tests/ui/parser/bad-let-else-statement.stderr +++ b/tests/ui/parser/bad-let-else-statement.stderr @@ -234,10 +234,10 @@ error: right curly brace `}` before `else` in a `let...else` statement not allow LL | let bad = asm! {" nop "} else { return; }; | ^ | -help: wrap the expression in parentheses +help: use parentheses instead of braces for this macro | -LL | let bad = (asm! {" nop "}) else { return; }; - | + + +LL | let bad = asm! (" nop ") else { return; }; + | ~ ~ error: right curly brace `}` before `else` in a `let...else` statement not allowed --> $DIR/bad-let-else-statement.rs:183:25 @@ -249,10 +249,10 @@ LL | b!(1); b!(2); | ----- in this macro invocation | = note: this error originates in the macro `b` (in Nightly builds, run with -Z macro-backtrace for more info) -help: wrap the expression in parentheses +help: use parentheses instead of braces for this macro | -LL | let x = (a! {}) else { return; }; - | + + +LL | let x = a! () else { return; }; + | ~~ error: aborting due to 19 previous errors