Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deny braced macro invocations in let-else #119062

Merged
merged 2 commits into from Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 7 additions & 3 deletions compiler/rustc_ast/src/util/classify.rs
Expand Up @@ -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
Expand Down Expand Up @@ -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(_) => {
compiler-errors marked this conversation as resolved.
Show resolved Hide resolved
// These should have been denied pre-expansion.
break None;
}

Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_parse/messages.ftl
Expand Up @@ -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 =
Expand Down
37 changes: 25 additions & 12 deletions compiler/rustc_parse/src/errors.rs
Expand Up @@ -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)]
Expand Down Expand Up @@ -936,7 +949,7 @@ pub(crate) struct InvalidExpressionInLetElse {
pub span: Span,
pub operator: &'static str,
#[subdiagnostic]
pub sugg: WrapExpressionInParentheses,
pub sugg: WrapInParentheses,
}

#[derive(Diagnostic)]
Expand All @@ -945,7 +958,7 @@ pub(crate) struct InvalidCurlyInLetElse {
#[primary_span]
pub span: Span,
#[subdiagnostic]
pub sugg: WrapExpressionInParentheses,
pub sugg: WrapInParentheses,
}

#[derive(Diagnostic)]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/expr.rs
Expand Up @@ -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(),
},
Expand Down
15 changes: 11 additions & 4 deletions compiler/rustc_parse/src/parser/stmt.rs
Expand Up @@ -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(),
},
Expand All @@ -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,
});
}
}
Expand Down
25 changes: 25 additions & 0 deletions tests/ui/parser/bad-let-else-statement.rs
Expand Up @@ -161,4 +161,29 @@ fn q() {
};
}

fn r() {
let ok = format_args!("") else { return; };

let bad = format_args! {""} 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() {}
28 changes: 27 additions & 1 deletion tests/ui/parser/bad-let-else-statement.stderr
Expand Up @@ -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:167:31
|
LL | let bad = format_args! {""} else { return; };
| ^
|
help: use parentheses instead of braces for this macro
|
LL | let bad = format_args! ("") else { return; };
| ~ ~

error: right curly brace `}` before `else` in a `let...else` statement not allowed
--> $DIR/bad-let-else-statement.rs:181: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: use parentheses instead of braces for this macro
|
LL | let x = a! () else { return; };
| ~~

error: aborting due to 19 previous errors