From 78bf7d846739c487698527706f5c152ba4ba4cb5 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sun, 22 Dec 2024 09:27:52 -0800 Subject: [PATCH] Implement asymmetrical precedence for closures and jumps --- compiler/rustc_ast/src/ast.rs | 15 ++- .../rustc_ast_pretty/src/pprust/state/expr.rs | 107 +++++++++--------- .../src/pprust/state/fixup.rs | 91 +++++++++++++-- tests/pretty/postfix-match/precedence.pp | 2 +- .../pprust-parenthesis-insertion.rs | 4 +- ..._legacy_const_generics-issue-123077.stderr | 4 +- tests/ui/unpretty/expanded-exhaustive.stdout | 2 +- 7 files changed, 150 insertions(+), 75 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 97e6879c33e99..eeebf09b9c305 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -1429,11 +1429,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 @@ -1490,6 +1494,7 @@ impl Expr { | ExprKind::Underscore | ExprKind::UnsafeBinderCast(..) | ExprKind::While(..) + | ExprKind::Yield(YieldKind::Postfix(..)) | ExprKind::Err(_) | ExprKind::Dummy => ExprPrecedence::Unambiguous, } diff --git a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs index df848a26d390a..3bce3080d6543 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs @@ -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; @@ -212,13 +212,6 @@ impl<'a> State<'a> { } fn print_expr_call(&mut self, func: &ast::Expr, args: &[P], 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 @@ -235,8 +228,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) } @@ -279,9 +280,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 // <::Type as Trait>::CONST + | BinOpKind::Lt // ::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), @@ -310,18 +326,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(), ); } @@ -342,8 +358,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(), ); } @@ -594,8 +610,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) => { @@ -608,8 +624,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) => { @@ -622,10 +638,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()); @@ -638,10 +655,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 { @@ -651,8 +669,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(), ); } } @@ -669,11 +687,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(), ); } } @@ -688,11 +705,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) => { @@ -701,21 +714,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`. @@ -765,11 +770,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)) => { diff --git a/compiler/rustc_ast_pretty/src/pprust/state/fixup.rs b/compiler/rustc_ast_pretty/src/pprust/state/fixup.rs index 3ef21f5cb29eb..eb5ac8b78a83d 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state/fixup.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state/fixup.rs @@ -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() } } diff --git a/tests/pretty/postfix-match/precedence.pp b/tests/pretty/postfix-match/precedence.pp index 967aa7bc39ef3..2052b445a2b30 100644 --- a/tests/pretty/postfix-match/precedence.pp +++ b/tests/pretty/postfix-match/precedence.pp @@ -26,7 +26,7 @@ _ => {} }; (4 as usize).match { _ => {} }; - (return).match { _ => {} }; + return.match { _ => {} }; (a = 42).match { _ => {} }; (|| {}).match { _ => {} }; (42..101).match { _ => {} }; diff --git a/tests/ui-fulldeps/pprust-parenthesis-insertion.rs b/tests/ui-fulldeps/pprust-parenthesis-insertion.rs index 2b41020d3071c..fa0eb02eb5630 100644 --- a/tests/ui-fulldeps/pprust-parenthesis-insertion.rs +++ b/tests/ui-fulldeps/pprust-parenthesis-insertion.rs @@ -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", diff --git a/tests/ui/invalid/invalid-rustc_legacy_const_generics-issue-123077.stderr b/tests/ui/invalid/invalid-rustc_legacy_const_generics-issue-123077.stderr index d92836aa063b1..8d120ae98d90d 100644 --- a/tests/ui/invalid/invalid-rustc_legacy_const_generics-issue-123077.stderr +++ b/tests/ui/invalid/invalid-rustc_legacy_const_generics-issue-123077.stderr @@ -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 diff --git a/tests/ui/unpretty/expanded-exhaustive.stdout b/tests/ui/unpretty/expanded-exhaustive.stdout index 19ae66f7a07aa..7d1d97baec54f 100644 --- a/tests/ui/unpretty/expanded-exhaustive.stdout +++ b/tests/ui/unpretty/expanded-exhaustive.stdout @@ -185,7 +185,7 @@ mod expressions { (static async || value); (static async move || value); || -> u8 { value }; - 1 + (|| {}); + 1 + || {}; } /// ExprKind::Block