Skip to content

Implement asymmetrical precedence for closures and jumps #134847

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+150 −75
Open

Implement asymmetrical precedence for closures and jumps #134847

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions compiler/rustc_ast/src/ast.rs
View file
Original file line number Diff line number Diff line change
@@ -1447,11 +1447,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
@@ -1508,6 +1512,7 @@ impl Expr {
| ExprKind::Underscore
| ExprKind::UnsafeBinderCast(..)
| ExprKind::While(..)
| ExprKind::Yield(YieldKind::Postfix(..))
| ExprKind::Err(_)
| ExprKind::Dummy => ExprPrecedence::Unambiguous,
}
107 changes: 54 additions & 53 deletions compiler/rustc_ast_pretty/src/pprust/state/expr.rs
View file
Original file line number Diff line number Diff line change
@@ -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)) => {
91 changes: 80 additions & 11 deletions compiler/rustc_ast_pretty/src/pprust/state/fixup.rs
View file
Original file line number Diff line number Diff line change
@@ -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.
///
/// 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.
///
/// 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 {
/// 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()
}
}
2 changes: 1 addition & 1 deletion tests/pretty/postfix-match/precedence.pp
View file
Original file line number Diff line number Diff line change
@@ -26,7 +26,7 @@
_ => {}
};
(4 as usize).match { _ => {} };
(return).match { _ => {} };
return.match { _ => {} };
(a = 42).match { _ => {} };
(|| {}).match { _ => {} };
(42..101).match { _ => {} };
4 changes: 2 additions & 2 deletions tests/ui-fulldeps/pprust-parenthesis-insertion.rs
View file
Original file line number Diff line number Diff line change
@@ -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",
4 changes: 2 additions & 2 deletions tests/ui/invalid/invalid-rustc_legacy_const_generics-issue-123077.stderr
View file
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion tests/ui/unpretty/exhaustive.expanded.stdout
View file
Original file line number Diff line number Diff line change
@@ -190,7 +190,7 @@ mod expressions {
(static async || value);
(static async move || value);
|| -> u8 { value };
1 + (|| {});
1 + || {};
}

/// ExprKind::Block