Rollup merge of #134847 - dtolnay:asymmetrical, r=fmease

Implement asymmetrical precedence for closures and jumps

I have been through a series of asymmetrical precedence designs in Syn, and finally have one that I like and is worth backporting into rustc. It is based on just 2 bits of state: `next_operator_can_begin_expr` and `next_operator_can_continue_expr`.

Asymmetrical precedence is the thing that enables `(return 1) + 1` to require parentheses while `1 + return 1` does not, despite `+` always having stronger precedence than `return` [according to the Rust Reference](https://doc.rust-lang.org/1.83.0/reference/expressions.html#expression-precedence). This is facilitated by `next_operator_can_continue_expr`.

Relatedly, it is the thing that enables `(return) - 1` to require parentheses while `return + 1` does not, despite `+` and `-` having exactly the same precedence. This is facilitated by `next_operator_can_begin_expr`.

**Example:**

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

fn main() {
    repro!{return}
    repro!{return 1}
}
```

`-Zunpretty=expanded` **Before:**

```console
fn main() {
    (return) - (return);
    (return) + (return);
    (return 1) - (return 1);
    (return 1) + (return 1);
}
```

**After:**

```console
fn main() {
    (return) - return;
    return + return;
    (return 1) - return 1;
    (return 1) + return 1;
}
```
This commit is contained in:
Matthias Krüger 2025-06-13 05:16:54 +02:00 committed by GitHub
commit b12bb2530b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 150 additions and 75 deletions

View file

@ -1441,11 +1441,15 @@ impl Expr {
}
}
ExprKind::Break(..)
| ExprKind::Ret(..)
| ExprKind::Yield(..)
| ExprKind::Yeet(..)
| ExprKind::Become(..) => ExprPrecedence::Jump,
ExprKind::Break(_ /*label*/, value)
| ExprKind::Ret(value)
| ExprKind::Yield(YieldKind::Prefix(value))
| ExprKind::Yeet(value) => match value {
Some(_) => ExprPrecedence::Jump,
None => ExprPrecedence::Unambiguous,
},
ExprKind::Become(_) => ExprPrecedence::Jump,
// `Range` claims to have higher precedence than `Assign`, but `x .. x = x` fails to
// parse, instead of parsing as `(x .. x) = x`. Giving `Range` a lower precedence
@ -1502,6 +1506,7 @@ impl Expr {
| ExprKind::Underscore
| ExprKind::UnsafeBinderCast(..)
| ExprKind::While(..)
| ExprKind::Yield(YieldKind::Postfix(..))
| ExprKind::Err(_)
| ExprKind::Dummy => ExprPrecedence::Unambiguous,
}

View file

@ -7,8 +7,8 @@ use rustc_ast::util::classify;
use rustc_ast::util::literal::escape_byte_str_symbol;
use rustc_ast::util::parser::{self, ExprPrecedence, Fixity};
use rustc_ast::{
self as ast, BlockCheckMode, FormatAlignment, FormatArgPosition, FormatArgsPiece, FormatCount,
FormatDebugHex, FormatSign, FormatTrait, YieldKind, token,
self as ast, BinOpKind, BlockCheckMode, FormatAlignment, FormatArgPosition, FormatArgsPiece,
FormatCount, FormatDebugHex, FormatSign, FormatTrait, YieldKind, token,
};
use crate::pp::Breaks::Inconsistent;
@ -214,13 +214,6 @@ impl<'a> State<'a> {
}
fn print_expr_call(&mut self, func: &ast::Expr, args: &[P<ast::Expr>], fixup: FixupContext) {
let needs_paren = match func.kind {
// In order to call a named field, needs parens: `(self.fun)()`
// But not for an unnamed field: `self.0()`
ast::ExprKind::Field(_, name) => !name.is_numeric(),
_ => func.precedence() < ExprPrecedence::Unambiguous,
};
// Independent of parenthesization related to precedence, we must
// parenthesize `func` if this is a statement context in which without
// parentheses, a statement boundary would occur inside `func` or
@ -237,8 +230,16 @@ impl<'a> State<'a> {
// because the latter is valid syntax but with the incorrect meaning.
// It's a match-expression followed by tuple-expression, not a function
// call.
self.print_expr_cond_paren(func, needs_paren, fixup.leftmost_subexpression());
let func_fixup = fixup.leftmost_subexpression_with_operator(true);
let needs_paren = match func.kind {
// In order to call a named field, needs parens: `(self.fun)()`
// But not for an unnamed field: `self.0()`
ast::ExprKind::Field(_, name) => !name.is_numeric(),
_ => func_fixup.precedence(func) < ExprPrecedence::Unambiguous,
};
self.print_expr_cond_paren(func, needs_paren, func_fixup);
self.print_call_post(args)
}
@ -281,9 +282,24 @@ impl<'a> State<'a> {
rhs: &ast::Expr,
fixup: FixupContext,
) {
let operator_can_begin_expr = match op {
| BinOpKind::Sub // -x
| BinOpKind::Mul // *x
| BinOpKind::And // &&x
| BinOpKind::Or // || x
| BinOpKind::BitAnd // &x
| BinOpKind::BitOr // |x| x
| BinOpKind::Shl // <<T as Trait>::Type as Trait>::CONST
| BinOpKind::Lt // <T as Trait>::CONST
=> true,
_ => false,
};
let left_fixup = fixup.leftmost_subexpression_with_operator(operator_can_begin_expr);
let binop_prec = op.precedence();
let left_prec = lhs.precedence();
let right_prec = rhs.precedence();
let left_prec = left_fixup.precedence(lhs);
let right_prec = fixup.precedence(rhs);
let (mut left_needs_paren, right_needs_paren) = match op.fixity() {
Fixity::Left => (left_prec < binop_prec, right_prec <= binop_prec),
@ -312,18 +328,18 @@ impl<'a> State<'a> {
_ => {}
}
self.print_expr_cond_paren(lhs, left_needs_paren, fixup.leftmost_subexpression());
self.print_expr_cond_paren(lhs, left_needs_paren, left_fixup);
self.space();
self.word_space(op.as_str());
self.print_expr_cond_paren(rhs, right_needs_paren, fixup.subsequent_subexpression());
self.print_expr_cond_paren(rhs, right_needs_paren, fixup.rightmost_subexpression());
}
fn print_expr_unary(&mut self, op: ast::UnOp, expr: &ast::Expr, fixup: FixupContext) {
self.word(op.as_str());
self.print_expr_cond_paren(
expr,
expr.precedence() < ExprPrecedence::Prefix,
fixup.subsequent_subexpression(),
fixup.precedence(expr) < ExprPrecedence::Prefix,
fixup.rightmost_subexpression(),
);
}
@ -344,8 +360,8 @@ impl<'a> State<'a> {
}
self.print_expr_cond_paren(
expr,
expr.precedence() < ExprPrecedence::Prefix,
fixup.subsequent_subexpression(),
fixup.precedence(expr) < ExprPrecedence::Prefix,
fixup.rightmost_subexpression(),
);
}
@ -590,8 +606,8 @@ impl<'a> State<'a> {
self.word_space("=");
self.print_expr_cond_paren(
rhs,
rhs.precedence() < ExprPrecedence::Assign,
fixup.subsequent_subexpression(),
fixup.precedence(rhs) < ExprPrecedence::Assign,
fixup.rightmost_subexpression(),
);
}
ast::ExprKind::AssignOp(op, lhs, rhs) => {
@ -604,8 +620,8 @@ impl<'a> State<'a> {
self.word_space(op.node.as_str());
self.print_expr_cond_paren(
rhs,
rhs.precedence() < ExprPrecedence::Assign,
fixup.subsequent_subexpression(),
fixup.precedence(rhs) < ExprPrecedence::Assign,
fixup.rightmost_subexpression(),
);
}
ast::ExprKind::Field(expr, ident) => {
@ -618,10 +634,11 @@ impl<'a> State<'a> {
self.print_ident(*ident);
}
ast::ExprKind::Index(expr, index, _) => {
let expr_fixup = fixup.leftmost_subexpression_with_operator(true);
self.print_expr_cond_paren(
expr,
expr.precedence() < ExprPrecedence::Unambiguous,
fixup.leftmost_subexpression(),
expr_fixup.precedence(expr) < ExprPrecedence::Unambiguous,
expr_fixup,
);
self.word("[");
self.print_expr(index, FixupContext::default());
@ -634,10 +651,11 @@ impl<'a> State<'a> {
// a "normal" binop gets parenthesized. (`LOr` is the lowest-precedence binop.)
let fake_prec = ExprPrecedence::LOr;
if let Some(e) = start {
let start_fixup = fixup.leftmost_subexpression_with_operator(true);
self.print_expr_cond_paren(
e,
e.precedence() < fake_prec,
fixup.leftmost_subexpression(),
start_fixup.precedence(e) < fake_prec,
start_fixup,
);
}
match limits {
@ -647,8 +665,8 @@ impl<'a> State<'a> {
if let Some(e) = end {
self.print_expr_cond_paren(
e,
e.precedence() < fake_prec,
fixup.subsequent_subexpression(),
fixup.precedence(e) < fake_prec,
fixup.rightmost_subexpression(),
);
}
}
@ -665,11 +683,10 @@ impl<'a> State<'a> {
self.space();
self.print_expr_cond_paren(
expr,
// Parenthesize if required by precedence, or in the
// case of `break 'inner: loop { break 'inner 1 } + 1`
expr.precedence() < ExprPrecedence::Jump
|| (opt_label.is_none() && classify::leading_labeled_expr(expr)),
fixup.subsequent_subexpression(),
// Parenthesize `break 'inner: loop { break 'inner 1 } + 1`
// ^---------------------------------^
opt_label.is_none() && classify::leading_labeled_expr(expr),
fixup.rightmost_subexpression(),
);
}
}
@ -684,11 +701,7 @@ impl<'a> State<'a> {
self.word("return");
if let Some(expr) = result {
self.word(" ");
self.print_expr_cond_paren(
expr,
expr.precedence() < ExprPrecedence::Jump,
fixup.subsequent_subexpression(),
);
self.print_expr(expr, fixup.rightmost_subexpression());
}
}
ast::ExprKind::Yeet(result) => {
@ -697,21 +710,13 @@ impl<'a> State<'a> {
self.word("yeet");
if let Some(expr) = result {
self.word(" ");
self.print_expr_cond_paren(
expr,
expr.precedence() < ExprPrecedence::Jump,
fixup.subsequent_subexpression(),
);
self.print_expr(expr, fixup.rightmost_subexpression());
}
}
ast::ExprKind::Become(result) => {
self.word("become");
self.word(" ");
self.print_expr_cond_paren(
result,
result.precedence() < ExprPrecedence::Jump,
fixup.subsequent_subexpression(),
);
self.print_expr(result, fixup.rightmost_subexpression());
}
ast::ExprKind::InlineAsm(a) => {
// FIXME: Print `builtin # asm` once macro `asm` uses `builtin_syntax`.
@ -761,11 +766,7 @@ impl<'a> State<'a> {
if let Some(expr) = e {
self.space();
self.print_expr_cond_paren(
expr,
expr.precedence() < ExprPrecedence::Jump,
fixup.subsequent_subexpression(),
);
self.print_expr(expr, fixup.rightmost_subexpression());
}
}
ast::ExprKind::Yield(YieldKind::Postfix(e)) => {

View file

@ -1,5 +1,6 @@
use rustc_ast::Expr;
use rustc_ast::util::{classify, parser};
use rustc_ast::util::classify;
use rustc_ast::util::parser::{self, ExprPrecedence};
use rustc_ast::{Expr, ExprKind, YieldKind};
// The default amount of fixing is minimal fixing, so all fixups are set to `false` by `Default`.
// Fixups should be turned on in a targeted fashion where needed.
@ -93,6 +94,24 @@ pub(crate) struct FixupContext {
/// }
/// ```
parenthesize_exterior_struct_lit: bool,
/// This is the difference between:
///
/// ```ignore (illustrative)
/// let _ = (return) - 1; // without paren, this would return -1
///
/// let _ = return + 1; // no paren because '+' cannot begin expr
/// ```
next_operator_can_begin_expr: bool,
/// This is the difference between:
///
/// ```ignore (illustrative)
/// let _ = 1 + return 1; // no parens if rightmost subexpression
///
/// let _ = 1 + (return 1) + 1; // needs parens
/// ```
next_operator_can_continue_expr: bool,
}
impl FixupContext {
@ -134,6 +153,8 @@ impl FixupContext {
match_arm: false,
leftmost_subexpression_in_match_arm: self.match_arm
|| self.leftmost_subexpression_in_match_arm,
next_operator_can_begin_expr: false,
next_operator_can_continue_expr: true,
..self
}
}
@ -148,19 +169,34 @@ impl FixupContext {
leftmost_subexpression_in_stmt: false,
match_arm: self.match_arm || self.leftmost_subexpression_in_match_arm,
leftmost_subexpression_in_match_arm: false,
next_operator_can_begin_expr: false,
next_operator_can_continue_expr: true,
..self
}
}
/// Transform this fixup into the one that should apply when printing any
/// subexpression that is neither a leftmost subexpression nor surrounded in
/// delimiters.
/// Transform this fixup into the one that should apply when printing a
/// leftmost subexpression followed by punctuation that is legal as the
/// first token of an expression.
pub(crate) fn leftmost_subexpression_with_operator(
self,
next_operator_can_begin_expr: bool,
) -> Self {
FixupContext { next_operator_can_begin_expr, ..self.leftmost_subexpression() }
}
/// Transform this fixup into the one that should apply when printing the
/// rightmost subexpression of the current expression.
///
/// This is for any subexpression that has a different first token than the
/// current expression, and is not surrounded by a paren/bracket/brace. For
/// example the `$b` in `$a + $b` and `-$b`, but not the one in `[$b]` or
/// `$a.f($b)`.
pub(crate) fn subsequent_subexpression(self) -> Self {
/// The rightmost subexpression is any subexpression that has a different
/// first token than the current expression, but has the same last token.
///
/// For example in `$a + $b` and `-$b`, the subexpression `$b` is a
/// rightmost subexpression.
///
/// Not every expression has a rightmost subexpression. For example neither
/// `[$b]` nor `$a.f($b)` have one.
pub(crate) fn rightmost_subexpression(self) -> Self {
FixupContext {
stmt: false,
leftmost_subexpression_in_stmt: false,
@ -193,6 +229,39 @@ impl FixupContext {
/// "let chain".
pub(crate) fn needs_par_as_let_scrutinee(self, expr: &Expr) -> bool {
self.parenthesize_exterior_struct_lit && parser::contains_exterior_struct_lit(expr)
|| parser::needs_par_as_let_scrutinee(expr.precedence())
|| parser::needs_par_as_let_scrutinee(self.precedence(expr))
}
/// Determines the effective precedence of a subexpression. Some expressions
/// have higher or lower precedence when adjacent to particular operators.
pub(crate) fn precedence(self, expr: &Expr) -> ExprPrecedence {
if self.next_operator_can_begin_expr {
// Decrease precedence of value-less jumps when followed by an
// operator that would otherwise get interpreted as beginning a
// value for the jump.
if let ExprKind::Break(..)
| ExprKind::Ret(..)
| ExprKind::Yeet(..)
| ExprKind::Yield(YieldKind::Prefix(..)) = expr.kind
{
return ExprPrecedence::Jump;
}
}
if !self.next_operator_can_continue_expr {
// Increase precedence of expressions that extend to the end of
// current statement or group.
if let ExprKind::Break(..)
| ExprKind::Closure(..)
| ExprKind::Ret(..)
| ExprKind::Yeet(..)
| ExprKind::Yield(YieldKind::Prefix(..))
| ExprKind::Range(None, ..) = expr.kind
{
return ExprPrecedence::Prefix;
}
}
expr.precedence()
}
}

View file

@ -26,7 +26,7 @@ pub fn main() {
_ => {}
};
(4 as usize).match { _ => {} };
(return).match { _ => {} };
return.match { _ => {} };
(a = 42).match { _ => {} };
(|| {}).match { _ => {} };
(42..101).match { _ => {} };

View file

@ -63,8 +63,8 @@ static EXPRS: &[&str] = &[
"(2 += 2) += 2",
// Return has lower precedence than a binary operator.
"(return 2) + 2",
"2 + (return 2)", // FIXME: no parenthesis needed.
"(return) + 2", // FIXME: no parenthesis needed.
"2 + return 2",
"return + 2",
// These mean different things.
"return - 2",
"(return) - 2",

View file

@ -19,7 +19,7 @@ LL | std::arch::x86_64::_mm_blend_ps(loop {}, loop {}, 5 + || ());
help: try using a const generic argument instead
|
LL - std::arch::x86_64::_mm_blend_ps(loop {}, loop {}, 5 + || ());
LL + std::arch::x86_64::_mm_blend_ps::<{ 5 + (|| ()) }>(loop {}, loop {});
LL + std::arch::x86_64::_mm_blend_ps::<{ 5 + || () }>(loop {}, loop {});
|
error: invalid argument to a legacy const generic: cannot have const blocks, closures, async blocks or items
@ -81,7 +81,7 @@ LL | std::arch::x86_64::_mm_inserti_si64(loop {}, loop {}, || (), 1 + || ())
help: try using a const generic argument instead
|
LL - std::arch::x86_64::_mm_inserti_si64(loop {}, loop {}, || (), 1 + || ());
LL + std::arch::x86_64::_mm_inserti_si64::<{ || () }, { 1 + (|| ()) }>(loop {}, loop {});
LL + std::arch::x86_64::_mm_inserti_si64::<{ || () }, { 1 + || () }>(loop {}, loop {});
|
error: aborting due to 7 previous errors

View file

@ -190,7 +190,7 @@ mod expressions {
(static async || value);
(static async move || value);
|| -> u8 { value };
1 + (|| {});
1 + || {};
}
/// ExprKind::Block