about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2024-07-02 17:47:45 +0200
committerGitHub <noreply@github.com>2024-07-02 17:47:45 +0200
commitf8f67b296915e70a67fbf4595b1f54d9d773be57 (patch)
tree0188cd7a6c3431559cc52d81c8a3bf37e743f2a8
parent7d97c59438e933e86f557ed999da3b8dfc6855a7 (diff)
parent06982239a6210cfc7b0b370634c9a9160c9b9dc4 (diff)
downloadrust-f8f67b296915e70a67fbf4595b1f54d9d773be57.tar.gz
rust-f8f67b296915e70a67fbf4595b1f54d9d773be57.zip
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.
-rw-r--r--compiler/rustc_ast/src/util/classify.rs79
-rw-r--r--compiler/rustc_ast_pretty/src/pprust/state/expr.rs8
-rw-r--r--tests/ui/unpretty/expanded-interpolation.rs20
-rw-r--r--tests/ui/unpretty/expanded-interpolation.stdout13
4 files changed, 117 insertions, 3 deletions
diff --git a/compiler/rustc_ast/src/util/classify.rs b/compiler/rustc_ast/src/util/classify.rs
index 4b2544ac47e..541b95ea971 100644
--- a/compiler/rustc_ast/src/util/classify.rs
+++ b/compiler/rustc_ast/src/util/classify.rs
@@ -1,7 +1,8 @@
 //! Routines the parser and pretty-printer use to classify AST nodes.
 
 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
 /// 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> {
     /// 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.
diff --git a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs
index 3d1f43a3766..5b13858f839 100644
--- a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs
+++ b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs
@@ -5,6 +5,7 @@ use ast::{ForLoopKind, MatchKind};
 use itertools::{Itertools, Position};
 use rustc_ast::ptr::P;
 use rustc_ast::token;
+use rustc_ast::util::classify;
 use rustc_ast::util::literal::escape_byte_str_symbol;
 use rustc_ast::util::parser::{self, AssocOp, Fixity};
 use rustc_ast::{self as ast, BlockCheckMode};
@@ -610,9 +611,12 @@ impl<'a> State<'a> {
                 }
                 if let Some(expr) = opt_expr {
                     self.space();
-                    self.print_expr_maybe_paren(
+                    self.print_expr_cond_paren(
                         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(),
                     );
                 }
diff --git a/tests/ui/unpretty/expanded-interpolation.rs b/tests/ui/unpretty/expanded-interpolation.rs
index 8f0e21ce870..1dc72c67f51 100644
--- a/tests/ui/unpretty/expanded-interpolation.rs
+++ b/tests/ui/unpretty/expanded-interpolation.rs
@@ -18,6 +18,26 @@ macro_rules! 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() {
     macro_rules! if_let {
         ($pat:pat, $expr:expr) => {
diff --git a/tests/ui/unpretty/expanded-interpolation.stdout b/tests/ui/unpretty/expanded-interpolation.stdout
index 73322b50f2d..556e57dbd92 100644
--- a/tests/ui/unpretty/expanded-interpolation.stdout
+++ b/tests/ui/unpretty/expanded-interpolation.stdout
@@ -20,6 +20,19 @@ macro_rules! expr { ($expr:expr) => { $expr }; }
 
 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() {
     macro_rules! if_let {
         ($pat:pat, $expr:expr) => { if let $pat = $expr {} };