Skip to content

Commit f8f67b2

Browse files
authored
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.
2 parents 7d97c59 + 0698223 commit f8f67b2

File tree

4 files changed

+117
-3
lines changed

4 files changed

+117
-3
lines changed

Diff for: compiler/rustc_ast/src/util/classify.rs

+78-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
//! Routines the parser and pretty-printer use to classify AST nodes.
22
33
use crate::ast::ExprKind::*;
4-
use crate::{ast, token::Delimiter};
4+
use crate::ast::{self, MatchKind};
5+
use crate::token::Delimiter;
56

67
/// This classification determines whether various syntactic positions break out
78
/// 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 {
8182
}
8283
}
8384

85+
/// Returns whether the leftmost token of the given expression is the label of a
86+
/// labeled loop or block, such as in `'inner: loop { break 'inner 1 } + 1`.
87+
///
88+
/// Such expressions are not allowed as the value of an unlabeled break.
89+
///
90+
/// ```ignore (illustrative)
91+
/// 'outer: {
92+
/// break 'inner: loop { break 'inner 1 } + 1; // invalid syntax
93+
///
94+
/// break 'outer 'inner: loop { break 'inner 1 } + 1; // okay
95+
///
96+
/// break ('inner: loop { break 'inner 1 } + 1); // okay
97+
///
98+
/// break ('inner: loop { break 'inner 1 }) + 1; // okay
99+
/// }
100+
/// ```
101+
pub fn leading_labeled_expr(mut expr: &ast::Expr) -> bool {
102+
loop {
103+
match &expr.kind {
104+
Block(_, label) | ForLoop { label, .. } | Loop(_, label, _) | While(_, _, label) => {
105+
return label.is_some();
106+
}
107+
108+
Assign(e, _, _)
109+
| AssignOp(_, e, _)
110+
| Await(e, _)
111+
| Binary(_, e, _)
112+
| Call(e, _)
113+
| Cast(e, _)
114+
| Field(e, _)
115+
| Index(e, _, _)
116+
| Match(e, _, MatchKind::Postfix)
117+
| Range(Some(e), _, _)
118+
| Try(e) => {
119+
expr = e;
120+
}
121+
MethodCall(method_call) => {
122+
expr = &method_call.receiver;
123+
}
124+
125+
AddrOf(..)
126+
| Array(..)
127+
| Become(..)
128+
| Break(..)
129+
| Closure(..)
130+
| ConstBlock(..)
131+
| Continue(..)
132+
| FormatArgs(..)
133+
| Gen(..)
134+
| If(..)
135+
| IncludedBytes(..)
136+
| InlineAsm(..)
137+
| Let(..)
138+
| Lit(..)
139+
| MacCall(..)
140+
| Match(_, _, MatchKind::Prefix)
141+
| OffsetOf(..)
142+
| Paren(..)
143+
| Path(..)
144+
| Range(None, _, _)
145+
| Repeat(..)
146+
| Ret(..)
147+
| Struct(..)
148+
| TryBlock(..)
149+
| Tup(..)
150+
| Type(..)
151+
| Unary(..)
152+
| Underscore
153+
| Yeet(..)
154+
| Yield(..)
155+
| Err(..)
156+
| Dummy => return false,
157+
}
158+
}
159+
}
160+
84161
pub enum TrailingBrace<'a> {
85162
/// Trailing brace in a macro call, like the one in `x as *const brace! {}`.
86163
/// We will suggest changing the macro call to a different delimiter.

Diff for: compiler/rustc_ast_pretty/src/pprust/state/expr.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use ast::{ForLoopKind, MatchKind};
55
use itertools::{Itertools, Position};
66
use rustc_ast::ptr::P;
77
use rustc_ast::token;
8+
use rustc_ast::util::classify;
89
use rustc_ast::util::literal::escape_byte_str_symbol;
910
use rustc_ast::util::parser::{self, AssocOp, Fixity};
1011
use rustc_ast::{self as ast, BlockCheckMode};
@@ -610,9 +611,12 @@ impl<'a> State<'a> {
610611
}
611612
if let Some(expr) = opt_expr {
612613
self.space();
613-
self.print_expr_maybe_paren(
614+
self.print_expr_cond_paren(
614615
expr,
615-
parser::PREC_JUMP,
616+
// Parenthesize if required by precedence, or in the
617+
// case of `break 'inner: loop { break 'inner 1 } + 1`
618+
expr.precedence().order() < parser::PREC_JUMP
619+
|| (opt_label.is_none() && classify::leading_labeled_expr(expr)),
616620
fixup.subsequent_subexpression(),
617621
);
618622
}

Diff for: tests/ui/unpretty/expanded-interpolation.rs

+20
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,26 @@ macro_rules! stmt {
1818
($stmt:stmt) => { $stmt };
1919
}
2020

21+
fn break_labeled_loop() {
22+
let no_paren = 'outer: loop {
23+
break 'outer expr!('inner: loop { break 'inner 1; } + 1);
24+
};
25+
26+
let paren_around_break_value = 'outer: loop {
27+
break expr!('inner: loop { break 'inner 1; } + 1);
28+
};
29+
30+
macro_rules! breaking {
31+
($value:expr) => {
32+
break $value
33+
};
34+
}
35+
36+
let paren_around_break_value = loop {
37+
breaking!('inner: loop { break 'inner 1; } + 1);
38+
};
39+
}
40+
2141
fn if_let() {
2242
macro_rules! if_let {
2343
($pat:pat, $expr:expr) => {

Diff for: tests/ui/unpretty/expanded-interpolation.stdout

+13
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,19 @@ macro_rules! expr { ($expr:expr) => { $expr }; }
2020

2121
macro_rules! stmt { ($stmt:stmt) => { $stmt }; }
2222

23+
fn break_labeled_loop() {
24+
let no_paren =
25+
'outer: loop { break 'outer 'inner: loop { break 'inner 1; } + 1; };
26+
27+
let paren_around_break_value =
28+
'outer: loop { break ('inner: loop { break 'inner 1; } + 1); };
29+
30+
macro_rules! breaking { ($value:expr) => { break $value }; }
31+
32+
let paren_around_break_value =
33+
loop { break ('inner: loop { break 'inner 1; } + 1); };
34+
}
35+
2336
fn if_let() {
2437
macro_rules! if_let {
2538
($pat:pat, $expr:expr) => { if let $pat = $expr {} };

0 commit comments

Comments
 (0)