Skip to content

Commit

Permalink
Fix redundant parens around braced macro call in match arms
Browse files Browse the repository at this point in the history
  • Loading branch information
dtolnay committed May 12, 2024
1 parent f895205 commit a9ec581
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 7 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_ast_pretty/src/pprust/state/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,7 @@ impl<'a> State<'a> {
}
_ => {
self.end(); // Close the ibox for the pattern.
self.print_expr(body, FixupContext::new_stmt());
self.print_expr(body, FixupContext::new_match_arm());
self.word(",");
}
}
Expand Down
57 changes: 52 additions & 5 deletions compiler/rustc_ast_pretty/src/pprust/state/fixup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,38 @@ pub(crate) struct FixupContext {
/// No parentheses required.
leftmost_subexpression_in_stmt: bool,

/// Print expression such that it can be parsed as a match arm.
///
/// This is almost equivalent to `stmt`, but the grammar diverges a tiny bit
/// between statements and match arms when it comes to braced macro calls.
/// Macro calls with brace delimiter terminate a statement without a
/// semicolon, but do not terminate a match-arm without comma.
///
/// ```ignore (illustrative)
/// m! {} - 1; // two statements: a macro call followed by -1 literal
///
/// match () {
/// _ => m! {} - 1, // binary subtraction operator
/// }
/// ```
match_arm: bool,

/// This is almost equivalent to `leftmost_subexpression_in_stmt`, other
/// than for braced macro calls.
///
/// If we have `m! {} - 1` as an expression, the leftmost subexpression
/// `m! {}` will need to be parenthesized in the statement case but not the
/// match-arm case.
///
/// ```ignore (illustrative)
/// (m! {}) - 1; // subexpression needs parens
///
/// match () {
/// _ => m! {} - 1, // no parens
/// }
/// ```
leftmost_subexpression_in_match_arm: bool,

/// This is the difference between:
///
/// ```ignore (illustrative)
Expand All @@ -68,6 +100,8 @@ impl Default for FixupContext {
FixupContext {
stmt: false,
leftmost_subexpression_in_stmt: false,
match_arm: false,
leftmost_subexpression_in_match_arm: false,
parenthesize_exterior_struct_lit: false,
}
}
Expand All @@ -76,13 +110,16 @@ impl Default for FixupContext {
impl FixupContext {
/// Create the initial fixup for printing an expression in statement
/// position.
///
/// This is currently also used for printing an expression as a match-arm,
/// but this is incorrect and leads to over-parenthesizing.
pub fn new_stmt() -> Self {
FixupContext { stmt: true, ..FixupContext::default() }
}

/// Create the initial fixup for printing an expression as the right-hand
/// side of a match arm.
pub fn new_match_arm() -> Self {
FixupContext { match_arm: true, ..FixupContext::default() }
}

/// Create the initial fixup for printing an expression as the "condition"
/// of an `if` or `while`. There are a few other positions which are
/// grammatically equivalent and also use this, such as the iterator
Expand All @@ -106,6 +143,9 @@ impl FixupContext {
FixupContext {
stmt: false,
leftmost_subexpression_in_stmt: self.stmt || self.leftmost_subexpression_in_stmt,
match_arm: false,
leftmost_subexpression_in_match_arm: self.match_arm
|| self.leftmost_subexpression_in_match_arm,
..self
}
}
Expand All @@ -119,7 +159,13 @@ impl FixupContext {
/// example the `$b` in `$a + $b` and `-$b`, but not the one in `[$b]` or
/// `$a.f($b)`.
pub fn subsequent_subexpression(self) -> Self {
FixupContext { stmt: false, leftmost_subexpression_in_stmt: false, ..self }
FixupContext {
stmt: false,
leftmost_subexpression_in_stmt: false,
match_arm: false,
leftmost_subexpression_in_match_arm: false,
..self
}
}

/// Determine whether parentheses are needed around the given expression to
Expand All @@ -128,7 +174,8 @@ impl FixupContext {
/// The documentation on `FixupContext::leftmost_subexpression_in_stmt` has
/// examples.
pub fn would_cause_statement_boundary(self, expr: &Expr) -> bool {
self.leftmost_subexpression_in_stmt && !classify::expr_requires_semi_to_be_stmt(expr)
(self.leftmost_subexpression_in_stmt && !classify::expr_requires_semi_to_be_stmt(expr))
|| (self.leftmost_subexpression_in_match_arm && classify::expr_is_complete(expr))
}

/// Determine whether parentheses are needed around the given `let`
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/macros/stringify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ fn test_expr() {
);
c2_match_arm!(
[ m! {} - 1 ],
"match () { _ => (m! {}) - 1, }", // parenthesis is redundant
"match () { _ => m! {} - 1, }",
"match () { _ => m! {} - 1 }",
);

Expand Down

0 comments on commit a9ec581

Please sign in to comment.