Rollup merge of #126883 - dtolnay:breakvalue, r=fmease

Parenthesize break values containing leading label

The AST pretty printer previously produced invalid syntax in the case of `break` expressions with a value that begins with a loop or block label.

```rust
macro_rules! expr {
    ($e:expr) => {
        $e
    };
}

fn main() {
    loop {
        break expr!('a: loop { break 'a 1; } + 1);
    };
}
```

`rustc -Zunpretty=expanded main.rs `:

```console
#![feature(prelude_import)]
#![no_std]
#[prelude_import]
use ::std::prelude::rust_2015::*;
#[macro_use]
extern crate std;
macro_rules! expr { ($e:expr) => { $e }; }

fn main() { loop { break 'a: loop { break 'a 1; } + 1; }; }
```

The expanded code is not valid Rust syntax. Printing invalid syntax is bad because it blocks `cargo expand` from being able to format the output as Rust syntax using rustfmt.

```console
error: parentheses are required around this expression to avoid confusion with a labeled break expression
 --> <anon>:9:26
  |
9 | fn main() { loop { break 'a: loop { break 'a 1; } + 1; }; }
  |                          ^^^^^^^^^^^^^^^^^^^^^^^^
  |
help: wrap the expression in parentheses
  |
9 | fn main() { loop { break ('a: loop { break 'a 1; }) + 1; }; }
  |                          +                        +
```

This PR updates the AST pretty-printer to insert parentheses around the value of a `break` expression as required to avoid this edge case.
This commit is contained in:
Matthias Krüger 2024-07-02 17:47:45 +02:00 committed by GitHub
commit f8f67b2969
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 117 additions and 3 deletions

View File

@ -1,7 +1,8 @@
//! Routines the parser and pretty-printer use to classify AST nodes. //! Routines the parser and pretty-printer use to classify AST nodes.
use crate::ast::ExprKind::*; use crate::ast::ExprKind::*;
use crate::{ast, token::Delimiter}; use crate::ast::{self, MatchKind};
use crate::token::Delimiter;
/// This classification determines whether various syntactic positions break out /// This classification determines whether various syntactic positions break out
/// of parsing the current expression (true) or continue parsing more of the /// of parsing the current expression (true) or continue parsing more of the
@ -81,6 +82,82 @@ pub fn expr_requires_semi_to_be_stmt(e: &ast::Expr) -> bool {
} }
} }
/// Returns whether the leftmost token of the given expression is the label of a
/// labeled loop or block, such as in `'inner: loop { break 'inner 1 } + 1`.
///
/// Such expressions are not allowed as the value of an unlabeled break.
///
/// ```ignore (illustrative)
/// 'outer: {
/// break 'inner: loop { break 'inner 1 } + 1; // invalid syntax
///
/// break 'outer 'inner: loop { break 'inner 1 } + 1; // okay
///
/// break ('inner: loop { break 'inner 1 } + 1); // okay
///
/// break ('inner: loop { break 'inner 1 }) + 1; // okay
/// }
/// ```
pub fn leading_labeled_expr(mut expr: &ast::Expr) -> bool {
loop {
match &expr.kind {
Block(_, label) | ForLoop { label, .. } | Loop(_, label, _) | While(_, _, label) => {
return label.is_some();
}
Assign(e, _, _)
| AssignOp(_, e, _)
| Await(e, _)
| Binary(_, e, _)
| Call(e, _)
| Cast(e, _)
| Field(e, _)
| Index(e, _, _)
| Match(e, _, MatchKind::Postfix)
| Range(Some(e), _, _)
| Try(e) => {
expr = e;
}
MethodCall(method_call) => {
expr = &method_call.receiver;
}
AddrOf(..)
| Array(..)
| Become(..)
| Break(..)
| Closure(..)
| ConstBlock(..)
| Continue(..)
| FormatArgs(..)
| Gen(..)
| If(..)
| IncludedBytes(..)
| InlineAsm(..)
| Let(..)
| Lit(..)
| MacCall(..)
| Match(_, _, MatchKind::Prefix)
| OffsetOf(..)
| Paren(..)
| Path(..)
| Range(None, _, _)
| Repeat(..)
| Ret(..)
| Struct(..)
| TryBlock(..)
| Tup(..)
| Type(..)
| Unary(..)
| Underscore
| Yeet(..)
| Yield(..)
| Err(..)
| Dummy => return false,
}
}
}
pub enum TrailingBrace<'a> { pub enum TrailingBrace<'a> {
/// Trailing brace in a macro call, like the one in `x as *const brace! {}`. /// Trailing brace in a macro call, like the one in `x as *const brace! {}`.
/// We will suggest changing the macro call to a different delimiter. /// We will suggest changing the macro call to a different delimiter.

View File

@ -5,6 +5,7 @@
use itertools::{Itertools, Position}; use itertools::{Itertools, Position};
use rustc_ast::ptr::P; use rustc_ast::ptr::P;
use rustc_ast::token; use rustc_ast::token;
use rustc_ast::util::classify;
use rustc_ast::util::literal::escape_byte_str_symbol; use rustc_ast::util::literal::escape_byte_str_symbol;
use rustc_ast::util::parser::{self, AssocOp, Fixity}; use rustc_ast::util::parser::{self, AssocOp, Fixity};
use rustc_ast::{self as ast, BlockCheckMode}; use rustc_ast::{self as ast, BlockCheckMode};
@ -610,9 +611,12 @@ pub(super) fn print_expr_outer_attr_style(
} }
if let Some(expr) = opt_expr { if let Some(expr) = opt_expr {
self.space(); self.space();
self.print_expr_maybe_paren( self.print_expr_cond_paren(
expr, expr,
parser::PREC_JUMP, // Parenthesize if required by precedence, or in the
// case of `break 'inner: loop { break 'inner 1 } + 1`
expr.precedence().order() < parser::PREC_JUMP
|| (opt_label.is_none() && classify::leading_labeled_expr(expr)),
fixup.subsequent_subexpression(), fixup.subsequent_subexpression(),
); );
} }

View File

@ -18,6 +18,26 @@ macro_rules! stmt {
($stmt:stmt) => { $stmt }; ($stmt:stmt) => { $stmt };
} }
fn break_labeled_loop() {
let no_paren = 'outer: loop {
break 'outer expr!('inner: loop { break 'inner 1; } + 1);
};
let paren_around_break_value = 'outer: loop {
break expr!('inner: loop { break 'inner 1; } + 1);
};
macro_rules! breaking {
($value:expr) => {
break $value
};
}
let paren_around_break_value = loop {
breaking!('inner: loop { break 'inner 1; } + 1);
};
}
fn if_let() { fn if_let() {
macro_rules! if_let { macro_rules! if_let {
($pat:pat, $expr:expr) => { ($pat:pat, $expr:expr) => {

View File

@ -20,6 +20,19 @@ macro_rules! expr { ($expr:expr) => { $expr }; }
macro_rules! stmt { ($stmt:stmt) => { $stmt }; } macro_rules! stmt { ($stmt:stmt) => { $stmt }; }
fn break_labeled_loop() {
let no_paren =
'outer: loop { break 'outer 'inner: loop { break 'inner 1; } + 1; };
let paren_around_break_value =
'outer: loop { break ('inner: loop { break 'inner 1; } + 1); };
macro_rules! breaking { ($value:expr) => { break $value }; }
let paren_around_break_value =
loop { break ('inner: loop { break 'inner 1; } + 1); };
}
fn if_let() { fn if_let() {
macro_rules! if_let { macro_rules! if_let {
($pat:pat, $expr:expr) => { if let $pat = $expr {} }; ($pat:pat, $expr:expr) => { if let $pat = $expr {} };