Skip to content

Commit 78bf7d8

Browse files
dtolnaydtolnay
committed
Implement asymmetrical precedence for closures and jumps
1 parent 17ffbc8 commit 78bf7d8

File tree

7 files changed

+150
-75
lines changed

7 files changed

+150
-75
lines changed

‎compiler/rustc_ast/src/ast.rs

+10-5
Original file line numberDiff line numberDiff line change
@@ -1429,11 +1429,15 @@ impl Expr {
14291429
}
14301430
}
14311431

1432-
ExprKind::Break(..)
1433-
| ExprKind::Ret(..)
1434-
| ExprKind::Yield(..)
1435-
| ExprKind::Yeet(..)
1436-
| ExprKind::Become(..) => ExprPrecedence::Jump,
1432+
ExprKind::Break(_ /*label*/, value)
1433+
| ExprKind::Ret(value)
1434+
| ExprKind::Yield(YieldKind::Prefix(value))
1435+
| ExprKind::Yeet(value) => match value {
1436+
Some(_) => ExprPrecedence::Jump,
1437+
None => ExprPrecedence::Unambiguous,
1438+
},
1439+
1440+
ExprKind::Become(_) => ExprPrecedence::Jump,
14371441

14381442
// `Range` claims to have higher precedence than `Assign`, but `x .. x = x` fails to
14391443
// parse, instead of parsing as `(x .. x) = x`. Giving `Range` a lower precedence
@@ -1490,6 +1494,7 @@ impl Expr {
14901494
| ExprKind::Underscore
14911495
| ExprKind::UnsafeBinderCast(..)
14921496
| ExprKind::While(..)
1497+
| ExprKind::Yield(YieldKind::Postfix(..))
14931498
| ExprKind::Err(_)
14941499
| ExprKind::Dummy => ExprPrecedence::Unambiguous,
14951500
}

‎compiler/rustc_ast_pretty/src/pprust/state/expr.rs

+54-53
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ use rustc_ast::util::classify;
77
use rustc_ast::util::literal::escape_byte_str_symbol;
88
use rustc_ast::util::parser::{self, ExprPrecedence, Fixity};
99
use rustc_ast::{
10-
self as ast, BlockCheckMode, FormatAlignment, FormatArgPosition, FormatArgsPiece, FormatCount,
11-
FormatDebugHex, FormatSign, FormatTrait, YieldKind, token,
10+
self as ast, BinOpKind, BlockCheckMode, FormatAlignment, FormatArgPosition, FormatArgsPiece,
11+
FormatCount, FormatDebugHex, FormatSign, FormatTrait, YieldKind, token,
1212
};
1313

1414
use crate::pp::Breaks::Inconsistent;
@@ -212,13 +212,6 @@ impl<'a> State<'a> {
212212
}
213213

214214
fn print_expr_call(&mut self, func: &ast::Expr, args: &[P<ast::Expr>], fixup: FixupContext) {
215-
let needs_paren = match func.kind {
216-
// In order to call a named field, needs parens: `(self.fun)()`
217-
// But not for an unnamed field: `self.0()`
218-
ast::ExprKind::Field(_, name) => !name.is_numeric(),
219-
_ => func.precedence() < ExprPrecedence::Unambiguous,
220-
};
221-
222215
// Independent of parenthesization related to precedence, we must
223216
// parenthesize `func` if this is a statement context in which without
224217
// parentheses, a statement boundary would occur inside `func` or
@@ -235,8 +228,16 @@ impl<'a> State<'a> {
235228
// because the latter is valid syntax but with the incorrect meaning.
236229
// It's a match-expression followed by tuple-expression, not a function
237230
// call.
238-
self.print_expr_cond_paren(func, needs_paren, fixup.leftmost_subexpression());
231+
let func_fixup = fixup.leftmost_subexpression_with_operator(true);
232+
233+
let needs_paren = match func.kind {
234+
// In order to call a named field, needs parens: `(self.fun)()`
235+
// But not for an unnamed field: `self.0()`
236+
ast::ExprKind::Field(_, name) => !name.is_numeric(),
237+
_ => func_fixup.precedence(func) < ExprPrecedence::Unambiguous,
238+
};
239239

240+
self.print_expr_cond_paren(func, needs_paren, func_fixup);
240241
self.print_call_post(args)
241242
}
242243

@@ -279,9 +280,24 @@ impl<'a> State<'a> {
279280
rhs: &ast::Expr,
280281
fixup: FixupContext,
281282
) {
283+
let operator_can_begin_expr = match op {
284+
| BinOpKind::Sub // -x
285+
| BinOpKind::Mul // *x
286+
| BinOpKind::And // &&x
287+
| BinOpKind::Or // || x
288+
| BinOpKind::BitAnd // &x
289+
| BinOpKind::BitOr // |x| x
290+
| BinOpKind::Shl // <<T as Trait>::Type as Trait>::CONST
291+
| BinOpKind::Lt // <T as Trait>::CONST
292+
=> true,
293+
_ => false,
294+
};
295+
296+
let left_fixup = fixup.leftmost_subexpression_with_operator(operator_can_begin_expr);
297+
282298
let binop_prec = op.precedence();
283-
let left_prec = lhs.precedence();
284-
let right_prec = rhs.precedence();
299+
let left_prec = left_fixup.precedence(lhs);
300+
let right_prec = fixup.precedence(rhs);
285301

286302
let (mut left_needs_paren, right_needs_paren) = match op.fixity() {
287303
Fixity::Left => (left_prec < binop_prec, right_prec <= binop_prec),
@@ -310,18 +326,18 @@ impl<'a> State<'a> {
310326
_ => {}
311327
}
312328

313-
self.print_expr_cond_paren(lhs, left_needs_paren, fixup.leftmost_subexpression());
329+
self.print_expr_cond_paren(lhs, left_needs_paren, left_fixup);
314330
self.space();
315331
self.word_space(op.as_str());
316-
self.print_expr_cond_paren(rhs, right_needs_paren, fixup.subsequent_subexpression());
332+
self.print_expr_cond_paren(rhs, right_needs_paren, fixup.rightmost_subexpression());
317333
}
318334

319335
fn print_expr_unary(&mut self, op: ast::UnOp, expr: &ast::Expr, fixup: FixupContext) {
320336
self.word(op.as_str());
321337
self.print_expr_cond_paren(
322338
expr,
323-
expr.precedence() < ExprPrecedence::Prefix,
324-
fixup.subsequent_subexpression(),
339+
fixup.precedence(expr) < ExprPrecedence::Prefix,
340+
fixup.rightmost_subexpression(),
325341
);
326342
}
327343

@@ -342,8 +358,8 @@ impl<'a> State<'a> {
342358
}
343359
self.print_expr_cond_paren(
344360
expr,
345-
expr.precedence() < ExprPrecedence::Prefix,
346-
fixup.subsequent_subexpression(),
361+
fixup.precedence(expr) < ExprPrecedence::Prefix,
362+
fixup.rightmost_subexpression(),
347363
);
348364
}
349365

@@ -594,8 +610,8 @@ impl<'a> State<'a> {
594610
self.word_space("=");
595611
self.print_expr_cond_paren(
596612
rhs,
597-
rhs.precedence() < ExprPrecedence::Assign,
598-
fixup.subsequent_subexpression(),
613+
fixup.precedence(rhs) < ExprPrecedence::Assign,
614+
fixup.rightmost_subexpression(),
599615
);
600616
}
601617
ast::ExprKind::AssignOp(op, lhs, rhs) => {
@@ -608,8 +624,8 @@ impl<'a> State<'a> {
608624
self.word_space(op.node.as_str());
609625
self.print_expr_cond_paren(
610626
rhs,
611-
rhs.precedence() < ExprPrecedence::Assign,
612-
fixup.subsequent_subexpression(),
627+
fixup.precedence(rhs) < ExprPrecedence::Assign,
628+
fixup.rightmost_subexpression(),
613629
);
614630
}
615631
ast::ExprKind::Field(expr, ident) => {
@@ -622,10 +638,11 @@ impl<'a> State<'a> {
622638
self.print_ident(*ident);
623639
}
624640
ast::ExprKind::Index(expr, index, _) => {
641+
let expr_fixup = fixup.leftmost_subexpression_with_operator(true);
625642
self.print_expr_cond_paren(
626643
expr,
627-
expr.precedence() < ExprPrecedence::Unambiguous,
628-
fixup.leftmost_subexpression(),
644+
expr_fixup.precedence(expr) < ExprPrecedence::Unambiguous,
645+
expr_fixup,
629646
);
630647
self.word("[");
631648
self.print_expr(index, FixupContext::default());
@@ -638,10 +655,11 @@ impl<'a> State<'a> {
638655
// a "normal" binop gets parenthesized. (`LOr` is the lowest-precedence binop.)
639656
let fake_prec = ExprPrecedence::LOr;
640657
if let Some(e) = start {
658+
let start_fixup = fixup.leftmost_subexpression_with_operator(true);
641659
self.print_expr_cond_paren(
642660
e,
643-
e.precedence() < fake_prec,
644-
fixup.leftmost_subexpression(),
661+
start_fixup.precedence(e) < fake_prec,
662+
start_fixup,
645663
);
646664
}
647665
match limits {
@@ -651,8 +669,8 @@ impl<'a> State<'a> {
651669
if let Some(e) = end {
652670
self.print_expr_cond_paren(
653671
e,
654-
e.precedence() < fake_prec,
655-
fixup.subsequent_subexpression(),
672+
fixup.precedence(e) < fake_prec,
673+
fixup.rightmost_subexpression(),
656674
);
657675
}
658676
}
@@ -669,11 +687,10 @@ impl<'a> State<'a> {
669687
self.space();
670688
self.print_expr_cond_paren(
671689
expr,
672-
// Parenthesize if required by precedence, or in the
673-
// case of `break 'inner: loop { break 'inner 1 } + 1`
674-
expr.precedence() < ExprPrecedence::Jump
675-
|| (opt_label.is_none() && classify::leading_labeled_expr(expr)),
676-
fixup.subsequent_subexpression(),
690+
// Parenthesize `break 'inner: loop { break 'inner 1 } + 1`
691+
// ^---------------------------------^
692+
opt_label.is_none() && classify::leading_labeled_expr(expr),
693+
fixup.rightmost_subexpression(),
677694
);
678695
}
679696
}
@@ -688,11 +705,7 @@ impl<'a> State<'a> {
688705
self.word("return");
689706
if let Some(expr) = result {
690707
self.word(" ");
691-
self.print_expr_cond_paren(
692-
expr,
693-
expr.precedence() < ExprPrecedence::Jump,
694-
fixup.subsequent_subexpression(),
695-
);
708+
self.print_expr(expr, fixup.rightmost_subexpression());
696709
}
697710
}
698711
ast::ExprKind::Yeet(result) => {
@@ -701,21 +714,13 @@ impl<'a> State<'a> {
701714
self.word("yeet");
702715
if let Some(expr) = result {
703716
self.word(" ");
704-
self.print_expr_cond_paren(
705-
expr,
706-
expr.precedence() < ExprPrecedence::Jump,
707-
fixup.subsequent_subexpression(),
708-
);
717+
self.print_expr(expr, fixup.rightmost_subexpression());
709718
}
710719
}
711720
ast::ExprKind::Become(result) => {
712721
self.word("become");
713722
self.word(" ");
714-
self.print_expr_cond_paren(
715-
result,
716-
result.precedence() < ExprPrecedence::Jump,
717-
fixup.subsequent_subexpression(),
718-
);
723+
self.print_expr(result, fixup.rightmost_subexpression());
719724
}
720725
ast::ExprKind::InlineAsm(a) => {
721726
// FIXME: Print `builtin # asm` once macro `asm` uses `builtin_syntax`.
@@ -765,11 +770,7 @@ impl<'a> State<'a> {
765770

766771
if let Some(expr) = e {
767772
self.space();
768-
self.print_expr_cond_paren(
769-
expr,
770-
expr.precedence() < ExprPrecedence::Jump,
771-
fixup.subsequent_subexpression(),
772-
);
773+
self.print_expr(expr, fixup.rightmost_subexpression());
773774
}
774775
}
775776
ast::ExprKind::Yield(YieldKind::Postfix(e)) => {

‎compiler/rustc_ast_pretty/src/pprust/state/fixup.rs

+80-11
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
use rustc_ast::Expr;
2-
use rustc_ast::util::{classify, parser};
1+
use rustc_ast::util::classify;
2+
use rustc_ast::util::parser::{self, ExprPrecedence};
3+
use rustc_ast::{Expr, ExprKind, YieldKind};
34

45
// The default amount of fixing is minimal fixing, so all fixups are set to `false` by `Default`.
56
// Fixups should be turned on in a targeted fashion where needed.
@@ -93,6 +94,24 @@ pub(crate) struct FixupContext {
9394
/// }
9495
/// ```
9596
parenthesize_exterior_struct_lit: bool,
97+
98+
/// This is the difference between:
99+
///
100+
/// ```ignore (illustrative)
101+
/// let _ = (return) - 1; // without paren, this would return -1
102+
///
103+
/// let _ = return + 1; // no paren because '+' cannot begin expr
104+
/// ```
105+
next_operator_can_begin_expr: bool,
106+
107+
/// This is the difference between:
108+
///
109+
/// ```ignore (illustrative)
110+
/// let _ = 1 + return 1; // no parens if rightmost subexpression
111+
///
112+
/// let _ = 1 + (return 1) + 1; // needs parens
113+
/// ```
114+
next_operator_can_continue_expr: bool,
96115
}
97116

98117
impl FixupContext {
@@ -134,6 +153,8 @@ impl FixupContext {
134153
match_arm: false,
135154
leftmost_subexpression_in_match_arm: self.match_arm
136155
|| self.leftmost_subexpression_in_match_arm,
156+
next_operator_can_begin_expr: false,
157+
next_operator_can_continue_expr: true,
137158
..self
138159
}
139160
}
@@ -148,19 +169,34 @@ impl FixupContext {
148169
leftmost_subexpression_in_stmt: false,
149170
match_arm: self.match_arm || self.leftmost_subexpression_in_match_arm,
150171
leftmost_subexpression_in_match_arm: false,
172+
next_operator_can_begin_expr: false,
173+
next_operator_can_continue_expr: true,
151174
..self
152175
}
153176
}
154177

155-
/// Transform this fixup into the one that should apply when printing any
156-
/// subexpression that is neither a leftmost subexpression nor surrounded in
157-
/// delimiters.
178+
/// Transform this fixup into the one that should apply when printing a
179+
/// leftmost subexpression followed by punctuation that is legal as the
180+
/// first token of an expression.
181+
pub(crate) fn leftmost_subexpression_with_operator(
182+
self,
183+
next_operator_can_begin_expr: bool,
184+
) -> Self {
185+
FixupContext { next_operator_can_begin_expr, ..self.leftmost_subexpression() }
186+
}
187+
188+
/// Transform this fixup into the one that should apply when printing the
189+
/// rightmost subexpression of the current expression.
190+
///
191+
/// The rightmost subexpression is any subexpression that has a different
192+
/// first token than the current expression, but has the same last token.
193+
///
194+
/// For example in `$a + $b` and `-$b`, the subexpression `$b` is a
195+
/// rightmost subexpression.
158196
///
159-
/// This is for any subexpression that has a different first token than the
160-
/// current expression, and is not surrounded by a paren/bracket/brace. For
161-
/// example the `$b` in `$a + $b` and `-$b`, but not the one in `[$b]` or
162-
/// `$a.f($b)`.
163-
pub(crate) fn subsequent_subexpression(self) -> Self {
197+
/// Not every expression has a rightmost subexpression. For example neither
198+
/// `[$b]` nor `$a.f($b)` have one.
199+
pub(crate) fn rightmost_subexpression(self) -> Self {
164200
FixupContext {
165201
stmt: false,
166202
leftmost_subexpression_in_stmt: false,
@@ -193,6 +229,39 @@ impl FixupContext {
193229
/// "let chain".
194230
pub(crate) fn needs_par_as_let_scrutinee(self, expr: &Expr) -> bool {
195231
self.parenthesize_exterior_struct_lit && parser::contains_exterior_struct_lit(expr)
196-
|| parser::needs_par_as_let_scrutinee(expr.precedence())
232+
|| parser::needs_par_as_let_scrutinee(self.precedence(expr))
233+
}
234+
235+
/// Determines the effective precedence of a subexpression. Some expressions
236+
/// have higher or lower precedence when adjacent to particular operators.
237+
pub(crate) fn precedence(self, expr: &Expr) -> ExprPrecedence {
238+
if self.next_operator_can_begin_expr {
239+
// Decrease precedence of value-less jumps when followed by an
240+
// operator that would otherwise get interpreted as beginning a
241+
// value for the jump.
242+
if let ExprKind::Break(..)
243+
| ExprKind::Ret(..)
244+
| ExprKind::Yeet(..)
245+
| ExprKind::Yield(YieldKind::Prefix(..)) = expr.kind
246+
{
247+
return ExprPrecedence::Jump;
248+
}
249+
}
250+
251+
if !self.next_operator_can_continue_expr {
252+
// Increase precedence of expressions that extend to the end of
253+
// current statement or group.
254+
if let ExprKind::Break(..)
255+
| ExprKind::Closure(..)
256+
| ExprKind::Ret(..)
257+
| ExprKind::Yeet(..)
258+
| ExprKind::Yield(YieldKind::Prefix(..))
259+
| ExprKind::Range(None, ..) = expr.kind
260+
{
261+
return ExprPrecedence::Prefix;
262+
}
263+
}
264+
265+
expr.precedence()
197266
}
198267
}

‎tests/pretty/postfix-match/precedence.pp

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
_ => {}
2727
};
2828
(4 as usize).match { _ => {} };
29-
(return).match { _ => {} };
29+
return.match { _ => {} };
3030
(a = 42).match { _ => {} };
3131
(|| {}).match { _ => {} };
3232
(42..101).match { _ => {} };

0 commit comments

Comments
 (0)