From 9cd47ea9f024bdc91c0bb9bd4d880eb8cc224215 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 6 Jul 2024 16:09:34 -0700 Subject: [PATCH 1/2] Add fixup test with closure that is not a rightmost subexpression --- tests/test_expr.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_expr.rs b/tests/test_expr.rs index 364386c5f8..a40c0f5d40 100644 --- a/tests/test_expr.rs +++ b/tests/test_expr.rs @@ -673,6 +673,7 @@ fn test_fixup() { quote! { if let _ = (a && b) && c {} }, quote! { if let _ = (S {}) {} }, quote! { break ('a: loop { break 'a 1 } + 1) }, + quote! { a + (|| b) + c }, ] { let original: Expr = syn::parse2(tokens).unwrap(); From 30abbd3f6e3053e65e2e4f51bf45d6d2f4da93e9 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 6 Jul 2024 16:12:10 -0700 Subject: [PATCH 2/2] Parenthesize closures and jumps which are not rightmost subexpressions --- src/expr.rs | 31 ++++++++++++++++++++++++++----- src/fixup.rs | 29 ++++++++++++++++++++++++++++- src/precedence.rs | 12 ------------ 3 files changed, 54 insertions(+), 18 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index e684068caa..2285084e15 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -3119,6 +3119,13 @@ pub(crate) mod printing { } } + fn precedence_of_rhs(e: &Expr, #[cfg(feature = "full")] fixup: FixupContext) -> Precedence { + #[cfg(feature = "full")] + return fixup.precedence_of_rhs(e); + #[cfg(not(feature = "full"))] + return Precedence::of(e); + } + #[cfg(feature = "full")] #[cfg_attr(docsrs, doc(cfg(feature = "printing")))] impl ToTokens for ExprArray { @@ -3150,7 +3157,7 @@ pub(crate) mod printing { e.eq_token.to_tokens(tokens); print_subexpression( &e.right, - Precedence::of_rhs(&e.right) < Precedence::Assign, + precedence_of_rhs(&e.right, fixup) < Precedence::Assign, tokens, fixup.subsequent_subexpression(), ); @@ -3209,7 +3216,11 @@ pub(crate) mod printing { let binop_prec = Precedence::of_binop(&e.op); let left_prec = Precedence::of(&e.left); - let right_prec = Precedence::of_rhs(&e.right); + let right_prec = precedence_of_rhs( + &e.right, + #[cfg(feature = "full")] + fixup, + ); let (mut left_needs_group, right_needs_group) = if let Precedence::Assign = binop_prec { (left_prec <= binop_prec, right_prec < binop_prec) } else { @@ -3696,7 +3707,7 @@ pub(crate) mod printing { if let Some(end) = &e.end { print_subexpression( end, - Precedence::of_rhs(end) <= Precedence::Range, + precedence_of_rhs(end, fixup) <= Precedence::Range, tokens, fixup.subsequent_subexpression(), ); @@ -3723,9 +3734,14 @@ pub(crate) mod printing { outer_attrs_to_tokens(&e.attrs, tokens); e.and_token.to_tokens(tokens); e.mutability.to_tokens(tokens); + let prec = precedence_of_rhs( + &e.expr, + #[cfg(feature = "full")] + fixup, + ); print_subexpression( &e.expr, - Precedence::of_rhs(&e.expr) < Precedence::Prefix, + prec < Precedence::Prefix, tokens, #[cfg(feature = "full")] fixup.subsequent_subexpression(), @@ -3844,9 +3860,14 @@ pub(crate) mod printing { ) { outer_attrs_to_tokens(&e.attrs, tokens); e.op.to_tokens(tokens); + let prec = precedence_of_rhs( + &e.expr, + #[cfg(feature = "full")] + fixup, + ); print_subexpression( &e.expr, - Precedence::of_rhs(&e.expr) < Precedence::Prefix, + prec < Precedence::Prefix, tokens, #[cfg(feature = "full")] fixup.subsequent_subexpression(), diff --git a/src/fixup.rs b/src/fixup.rs index 5407c9fdf3..4b9fe5644c 100644 --- a/src/fixup.rs +++ b/src/fixup.rs @@ -85,6 +85,14 @@ pub(crate) struct FixupContext { // } // parenthesize_exterior_struct_lit: bool, + + // This is the difference between: + // + // let _ = 1 + return 1; // no parens if rightmost subexpression + // + // let _ = 1 + (return 1) + 1; // needs parens + // + parenthesize_exterior_jump: bool, } impl FixupContext { @@ -96,6 +104,7 @@ impl FixupContext { match_arm: false, leftmost_subexpression_in_match_arm: false, parenthesize_exterior_struct_lit: false, + parenthesize_exterior_jump: false, }; /// Create the initial fixup for printing an expression in statement @@ -145,6 +154,7 @@ impl FixupContext { match_arm: false, leftmost_subexpression_in_match_arm: self.match_arm || self.leftmost_subexpression_in_match_arm, + parenthesize_exterior_jump: true, ..self } } @@ -159,6 +169,7 @@ impl FixupContext { leftmost_subexpression_in_stmt: false, match_arm: self.match_arm || self.leftmost_subexpression_in_match_arm, leftmost_subexpression_in_match_arm: false, + parenthesize_exterior_jump: true, ..self } } @@ -205,7 +216,23 @@ impl FixupContext { /// "let chain". pub fn needs_group_as_let_scrutinee(self, expr: &Expr) -> bool { self.parenthesize_exterior_struct_lit && classify::confusable_with_adjacent_block(expr) - || Precedence::of_rhs(expr) <= Precedence::And + || self.precedence_of_rhs(expr) <= Precedence::And + } + + /// Determines the effective precedence of a subexpression. Some expressions + /// have higher precedence on the right side of a binary operator than on + /// the left. + pub fn precedence_of_rhs(self, expr: &Expr) -> Precedence { + if !self.parenthesize_exterior_jump { + match expr { + Expr::Break(_) | Expr::Closure(_) | Expr::Return(_) | Expr::Yield(_) => { + return Precedence::Prefix; + } + Expr::Range(e) if e.start.is_none() => return Precedence::Prefix, + _ => {} + } + } + Precedence::of(expr) } } diff --git a/src/precedence.rs b/src/precedence.rs index a708e291d0..14320be80a 100644 --- a/src/precedence.rs +++ b/src/precedence.rs @@ -122,18 +122,6 @@ impl Precedence { Expr::Closure(_) => unreachable!(), } } - - #[cfg(feature = "printing")] - pub(crate) fn of_rhs(e: &Expr) -> Self { - match e { - Expr::Break(_) | Expr::Closure(_) | Expr::Return(_) | Expr::Yield(_) => { - Precedence::Prefix - } - #[cfg(feature = "full")] - Expr::Range(e) if e.start.is_none() => Precedence::Prefix, - _ => Precedence::of(e), - } - } } impl Copy for Precedence {}