Skip to content

Commit 7764444

Browse files
dtolnaydtolnay
committed
Implement asymmetrical precedence for closures and jumps
1 parent ce36a96 commit 7764444

File tree

7 files changed

+149
-75
lines changed

7 files changed

+149
-75
lines changed

Diff for: ‎compiler/rustc_ast/src/ast.rs

+9-5
Original file line numberDiff line numberDiff line change
@@ -1319,11 +1319,15 @@ impl Expr {
13191319
}
13201320
}
13211321

1322-
ExprKind::Break(..)
1323-
| ExprKind::Ret(..)
1324-
| ExprKind::Yield(..)
1325-
| ExprKind::Yeet(..)
1326-
| ExprKind::Become(..) => ExprPrecedence::Jump,
1322+
ExprKind::Break(_ /*label*/, value)
1323+
| ExprKind::Ret(value)
1324+
| ExprKind::Yield(value)
1325+
| ExprKind::Yeet(value) => match value {
1326+
Some(_) => ExprPrecedence::Jump,
1327+
None => ExprPrecedence::Unambiguous,
1328+
},
1329+
1330+
ExprKind::Become(_) => ExprPrecedence::Jump,
13271331

13281332
// `Range` claims to have higher precedence than `Assign`, but `x .. x = x` fails to
13291333
// parse, instead of parsing as `(x .. x) = x`. Giving `Range` a lower precedence

Diff for: ‎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, AssocOp, ExprPrecedence, Fixity};
99
use rustc_ast::{
10-
self as ast, BlockCheckMode, FormatAlignment, FormatArgPosition, FormatArgsPiece, FormatCount,
11-
FormatDebugHex, FormatSign, FormatTrait, token,
10+
self as ast, BinOpKind, BlockCheckMode, FormatAlignment, FormatArgPosition, FormatArgsPiece,
11+
FormatCount, FormatDebugHex, FormatSign, FormatTrait, 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,10 +280,25 @@ impl<'a> State<'a> {
279280
rhs: &ast::Expr,
280281
fixup: FixupContext,
281282
) {
283+
let operator_can_begin_expr = match op.node {
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 assoc_op = AssocOp::from_ast_binop(op.node);
283299
let binop_prec = assoc_op.precedence();
284-
let left_prec = lhs.precedence();
285-
let right_prec = rhs.precedence();
300+
let left_prec = left_fixup.precedence(lhs);
301+
let right_prec = fixup.precedence(rhs);
286302

287303
let (mut left_needs_paren, right_needs_paren) = match assoc_op.fixity() {
288304
Fixity::Left => (left_prec < binop_prec, right_prec <= binop_prec),
@@ -311,18 +327,18 @@ impl<'a> State<'a> {
311327
_ => {}
312328
}
313329

314-
self.print_expr_cond_paren(lhs, left_needs_paren, fixup.leftmost_subexpression());
330+
self.print_expr_cond_paren(lhs, left_needs_paren, left_fixup);
315331
self.space();
316332
self.word_space(op.node.as_str());
317-
self.print_expr_cond_paren(rhs, right_needs_paren, fixup.subsequent_subexpression());
333+
self.print_expr_cond_paren(rhs, right_needs_paren, fixup.rightmost_subexpression());
318334
}
319335

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

@@ -343,8 +359,8 @@ impl<'a> State<'a> {
343359
}
344360
self.print_expr_cond_paren(
345361
expr,
346-
expr.precedence() < ExprPrecedence::Prefix,
347-
fixup.subsequent_subexpression(),
362+
fixup.precedence(expr) < ExprPrecedence::Prefix,
363+
fixup.rightmost_subexpression(),
348364
);
349365
}
350366

@@ -587,8 +603,8 @@ impl<'a> State<'a> {
587603
self.word_space("=");
588604
self.print_expr_cond_paren(
589605
rhs,
590-
rhs.precedence() < ExprPrecedence::Assign,
591-
fixup.subsequent_subexpression(),
606+
fixup.precedence(rhs) < ExprPrecedence::Assign,
607+
fixup.rightmost_subexpression(),
592608
);
593609
}
594610
ast::ExprKind::AssignOp(op, lhs, rhs) => {
@@ -602,8 +618,8 @@ impl<'a> State<'a> {
602618
self.word_space("=");
603619
self.print_expr_cond_paren(
604620
rhs,
605-
rhs.precedence() < ExprPrecedence::Assign,
606-
fixup.subsequent_subexpression(),
621+
fixup.precedence(rhs) < ExprPrecedence::Assign,
622+
fixup.rightmost_subexpression(),
607623
);
608624
}
609625
ast::ExprKind::Field(expr, ident) => {
@@ -616,10 +632,11 @@ impl<'a> State<'a> {
616632
self.print_ident(*ident);
617633
}
618634
ast::ExprKind::Index(expr, index, _) => {
635+
let expr_fixup = fixup.leftmost_subexpression_with_operator(true);
619636
self.print_expr_cond_paren(
620637
expr,
621-
expr.precedence() < ExprPrecedence::Unambiguous,
622-
fixup.leftmost_subexpression(),
638+
expr_fixup.precedence(expr) < ExprPrecedence::Unambiguous,
639+
expr_fixup,
623640
);
624641
self.word("[");
625642
self.print_expr(index, FixupContext::default());
@@ -632,10 +649,11 @@ impl<'a> State<'a> {
632649
// a "normal" binop gets parenthesized. (`LOr` is the lowest-precedence binop.)
633650
let fake_prec = ExprPrecedence::LOr;
634651
if let Some(e) = start {
652+
let start_fixup = fixup.leftmost_subexpression_with_operator(true);
635653
self.print_expr_cond_paren(
636654
e,
637-
e.precedence() < fake_prec,
638-
fixup.leftmost_subexpression(),
655+
start_fixup.precedence(e) < fake_prec,
656+
start_fixup,
639657
);
640658
}
641659
match limits {
@@ -645,8 +663,8 @@ impl<'a> State<'a> {
645663
if let Some(e) = end {
646664
self.print_expr_cond_paren(
647665
e,
648-
e.precedence() < fake_prec,
649-
fixup.subsequent_subexpression(),
666+
fixup.precedence(e) < fake_prec,
667+
fixup.rightmost_subexpression(),
650668
);
651669
}
652670
}
@@ -663,11 +681,10 @@ impl<'a> State<'a> {
663681
self.space();
664682
self.print_expr_cond_paren(
665683
expr,
666-
// Parenthesize if required by precedence, or in the
667-
// case of `break 'inner: loop { break 'inner 1 } + 1`
668-
expr.precedence() < ExprPrecedence::Jump
669-
|| (opt_label.is_none() && classify::leading_labeled_expr(expr)),
670-
fixup.subsequent_subexpression(),
684+
// Parenthesize `break 'inner: loop { break 'inner 1 } + 1`
685+
// ^---------------------------------^
686+
opt_label.is_none() && classify::leading_labeled_expr(expr),
687+
fixup.rightmost_subexpression(),
671688
);
672689
}
673690
}
@@ -682,11 +699,7 @@ impl<'a> State<'a> {
682699
self.word("return");
683700
if let Some(expr) = result {
684701
self.word(" ");
685-
self.print_expr_cond_paren(
686-
expr,
687-
expr.precedence() < ExprPrecedence::Jump,
688-
fixup.subsequent_subexpression(),
689-
);
702+
self.print_expr(expr, fixup.rightmost_subexpression());
690703
}
691704
}
692705
ast::ExprKind::Yeet(result) => {
@@ -695,21 +708,13 @@ impl<'a> State<'a> {
695708
self.word("yeet");
696709
if let Some(expr) = result {
697710
self.word(" ");
698-
self.print_expr_cond_paren(
699-
expr,
700-
expr.precedence() < ExprPrecedence::Jump,
701-
fixup.subsequent_subexpression(),
702-
);
711+
self.print_expr(expr, fixup.rightmost_subexpression());
703712
}
704713
}
705714
ast::ExprKind::Become(result) => {
706715
self.word("become");
707716
self.word(" ");
708-
self.print_expr_cond_paren(
709-
result,
710-
result.precedence() < ExprPrecedence::Jump,
711-
fixup.subsequent_subexpression(),
712-
);
717+
self.print_expr(result, fixup.rightmost_subexpression());
713718
}
714719
ast::ExprKind::InlineAsm(a) => {
715720
// FIXME: Print `builtin # asm` once macro `asm` uses `builtin_syntax`.
@@ -759,11 +764,7 @@ impl<'a> State<'a> {
759764

760765
if let Some(expr) = e {
761766
self.space();
762-
self.print_expr_cond_paren(
763-
expr,
764-
expr.precedence() < ExprPrecedence::Jump,
765-
fixup.subsequent_subexpression(),
766-
);
767+
self.print_expr(expr, fixup.rightmost_subexpression());
767768
}
768769
}
769770
ast::ExprKind::Try(e) => {

Diff for: ‎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};
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(..) = 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(..)
259+
| ExprKind::Range(None, ..) = expr.kind
260+
{
261+
return ExprPrecedence::Prefix;
262+
}
263+
}
264+
265+
expr.precedence()
197266
}
198267
}

Diff for: ‎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 { _ => {} };

Diff for: ‎tests/ui-fulldeps/pprust-parenthesis-insertion.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ static EXPRS: &[&str] = &[
6363
"(2 += 2) += 2",
6464
// Return has lower precedence than a binary operator.
6565
"(return 2) + 2",
66-
"2 + (return 2)", // FIXME: no parenthesis needed.
67-
"(return) + 2", // FIXME: no parenthesis needed.
66+
"2 + return 2",
67+
"return + 2",
6868
// These mean different things.
6969
"return - 2",
7070
"(return) - 2",

0 commit comments

Comments
 (0)