Give a name to each distinct manipulation of pretty-printer FixupContext

compiler/rustc_ast_pretty/src/pprust/state.rs

@@ -16,7 +16,6 @@ use rustc_ast::token::{self, BinOpToken, CommentKind, Delimiter, Nonterminal, To
 use rustc_ast::tokenstream::{Spacing, TokenStream, TokenTree};
 use rustc_ast::util::classify;
 use rustc_ast::util::comments::{Comment, CommentStyle};
-use rustc_ast::util::parser;
 use rustc_ast::{self as ast, AttrArgs, AttrArgsEq, BlockCheckMode, PatKind};
 use rustc_ast::{attr, BindingMode, ByRef, DelimArgs, RangeEnd, RangeSyntax, Term};
 use rustc_ast::{GenericArg, GenericBound, SelfKind};
@@ -1253,22 +1252,14 @@ impl<'a> State<'a> {
             ast::StmtKind::Item(item) => self.print_item(item),
             ast::StmtKind::Expr(expr) => {
                 self.space_if_not_bol();
-                self.print_expr_outer_attr_style(
+                self.print_expr_outer_attr_style(expr, false, FixupContext::new_stmt());
-                    expr,
-                    false,
-                    FixupContext { stmt: true, ..FixupContext::default() },
-                );
                 if classify::expr_requires_semi_to_be_stmt(expr) {
                     self.word(";");
                 }
             }
             ast::StmtKind::Semi(expr) => {
                 self.space_if_not_bol();
-                self.print_expr_outer_attr_style(
+                self.print_expr_outer_attr_style(expr, false, FixupContext::new_stmt());
-                    expr,
-                    false,
-                    FixupContext { stmt: true, ..FixupContext::default() },
-                );
                 self.word(";");
             }
             ast::StmtKind::Empty => {
@@ -1320,11 +1311,7 @@ impl<'a> State<'a> {
                 ast::StmtKind::Expr(expr) if i == blk.stmts.len() - 1 => {
                     self.maybe_print_comment(st.span.lo());
                     self.space_if_not_bol();
-                    self.print_expr_outer_attr_style(
+                    self.print_expr_outer_attr_style(expr, false, FixupContext::new_stmt());
-                        expr,
-                        false,
-                        FixupContext { stmt: true, ..FixupContext::default() },
-                    );
                     self.maybe_print_trailing_comment(expr.span, Some(blk.span.hi()));
                 }
                 _ => self.print_stmt(st),
@@ -1368,8 +1355,7 @@ impl<'a> State<'a> {
         self.word_space("=");
         self.print_expr_cond_paren(
             expr,
-            fixup.parenthesize_exterior_struct_lit && parser::contains_exterior_struct_lit(expr)
+            fixup.needs_par_as_let_scrutinee(expr),
-                || parser::needs_par_as_let_scrutinee(expr.precedence().order()),
             FixupContext::default(),
         );
     }

compiler/rustc_ast_pretty/src/pprust/state/expr.rs

@@ -5,7 +5,6 @@ use ast::{ForLoopKind, MatchKind};
 use itertools::{Itertools, Position};
 use rustc_ast::ptr::P;
 use rustc_ast::token;
-use rustc_ast::util::classify;
 use rustc_ast::util::literal::escape_byte_str_symbol;
 use rustc_ast::util::parser::{self, AssocOp, Fixity};
 use rustc_ast::{self as ast, BlockCheckMode};
@@ -65,9 +64,7 @@ impl<'a> State<'a> {
     /// Prints an expr using syntax that's acceptable in a condition position, such as the `cond` in
     /// `if cond { ... }`.
     fn print_expr_as_cond(&mut self, expr: &ast::Expr) {
-        let fixup =
+        self.print_expr_cond_paren(expr, Self::cond_needs_par(expr), FixupContext::new_cond())
-            FixupContext { parenthesize_exterior_struct_lit: true, ..FixupContext::default() };
-        self.print_expr_cond_paren(expr, Self::cond_needs_par(expr), fixup)
     }
 
     /// Does `expr` need parentheses when printed in a condition position?
@@ -239,15 +236,7 @@ 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_maybe_paren(
+        self.print_expr_maybe_paren(func, prec, fixup.leftmost_subexpression());
-            func,
-            prec,
-            FixupContext {
-                stmt: false,
-                leftmost_subexpression_in_stmt: fixup.stmt || fixup.leftmost_subexpression_in_stmt,
-                ..fixup
-            },
-        );
 
         self.print_call_post(args)
     }
@@ -316,33 +305,17 @@ impl<'a> State<'a> {
             _ => left_prec,
         };
 
-        self.print_expr_maybe_paren(
+        self.print_expr_maybe_paren(lhs, left_prec, fixup.leftmost_subexpression());
-            lhs,
-            left_prec,
-            FixupContext {
-                stmt: false,
-                leftmost_subexpression_in_stmt: fixup.stmt || fixup.leftmost_subexpression_in_stmt,
-                ..fixup
-            },
-        );
 
         self.space();
         self.word_space(op.node.as_str());
 
-        self.print_expr_maybe_paren(
+        self.print_expr_maybe_paren(rhs, right_prec, fixup.subsequent_subexpression());
-            rhs,
-            right_prec,
-            FixupContext { stmt: false, leftmost_subexpression_in_stmt: false, ..fixup },
-        );
     }
 
     fn print_expr_unary(&mut self, op: ast::UnOp, expr: &ast::Expr, fixup: FixupContext) {
         self.word(op.as_str());
-        self.print_expr_maybe_paren(
+        self.print_expr_maybe_paren(expr, parser::PREC_PREFIX, fixup.subsequent_subexpression());
-            expr,
-            parser::PREC_PREFIX,
-            FixupContext { stmt: false, leftmost_subexpression_in_stmt: false, ..fixup },
-        );
     }
 
     fn print_expr_addr_of(
@@ -360,11 +333,7 @@ impl<'a> State<'a> {
                 self.print_mutability(mutability, true);
             }
         }
-        self.print_expr_maybe_paren(
+        self.print_expr_maybe_paren(expr, parser::PREC_PREFIX, fixup.subsequent_subexpression());
-            expr,
-            parser::PREC_PREFIX,
-            FixupContext { stmt: false, leftmost_subexpression_in_stmt: false, ..fixup },
-        );
     }
 
     pub(super) fn print_expr(&mut self, expr: &ast::Expr, fixup: FixupContext) {
@@ -399,8 +368,7 @@ impl<'a> State<'a> {
         //
         // Same applies to a small set of other expression kinds which eagerly
         // terminate a statement which opens with them.
-        let needs_par =
+        let needs_par = fixup.would_cause_statement_boundary(expr);
-            fixup.leftmost_subexpression_in_stmt && !classify::expr_requires_semi_to_be_stmt(expr);
         if needs_par {
             self.popen();
             fixup = FixupContext::default();
@@ -448,16 +416,7 @@ impl<'a> State<'a> {
             }
             ast::ExprKind::Cast(expr, ty) => {
                 let prec = AssocOp::As.precedence() as i8;
-                self.print_expr_maybe_paren(
+                self.print_expr_maybe_paren(expr, prec, fixup.leftmost_subexpression());
-                    expr,
-                    prec,
-                    FixupContext {
-                        stmt: false,
-                        leftmost_subexpression_in_stmt: fixup.stmt
-                            || fixup.leftmost_subexpression_in_stmt,
-                        ..fixup
-                    },
-                );
                 self.space();
                 self.word_space("as");
                 self.print_type(ty);
@@ -589,70 +548,34 @@ impl<'a> State<'a> {
                 self.print_block_with_attrs(blk, attrs);
             }
             ast::ExprKind::Await(expr, _) => {
-                // Same fixups as ExprKind::MethodCall.
                 self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX, fixup);
                 self.word(".await");
             }
             ast::ExprKind::Assign(lhs, rhs, _) => {
-                // Same fixups as ExprKind::Binary.
                 let prec = AssocOp::Assign.precedence() as i8;
-                self.print_expr_maybe_paren(
+                self.print_expr_maybe_paren(lhs, prec + 1, fixup.leftmost_subexpression());
-                    lhs,
-                    prec + 1,
-                    FixupContext {
-                        stmt: false,
-                        leftmost_subexpression_in_stmt: fixup.stmt
-                            || fixup.leftmost_subexpression_in_stmt,
-                        ..fixup
-                    },
-                );
                 self.space();
                 self.word_space("=");
-                self.print_expr_maybe_paren(
+                self.print_expr_maybe_paren(rhs, prec, fixup.subsequent_subexpression());
-                    rhs,
-                    prec,
-                    FixupContext { stmt: false, leftmost_subexpression_in_stmt: false, ..fixup },
-                );
             }
             ast::ExprKind::AssignOp(op, lhs, rhs) => {
-                // Same fixups as ExprKind::Binary.
                 let prec = AssocOp::Assign.precedence() as i8;
-                self.print_expr_maybe_paren(
+                self.print_expr_maybe_paren(lhs, prec + 1, fixup.leftmost_subexpression());
-                    lhs,
-                    prec + 1,
-                    FixupContext {
-                        stmt: false,
-                        leftmost_subexpression_in_stmt: fixup.stmt
-                            || fixup.leftmost_subexpression_in_stmt,
-                        ..fixup
-                    },
-                );
                 self.space();
                 self.word(op.node.as_str());
                 self.word_space("=");
-                self.print_expr_maybe_paren(
+                self.print_expr_maybe_paren(rhs, prec, fixup.subsequent_subexpression());
-                    rhs,
-                    prec,
-                    FixupContext { stmt: false, leftmost_subexpression_in_stmt: false, ..fixup },
-                );
             }
             ast::ExprKind::Field(expr, ident) => {
-                // Same fixups as ExprKind::MethodCall.
                 self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX, fixup);
                 self.word(".");
                 self.print_ident(*ident);
             }
             ast::ExprKind::Index(expr, index, _) => {
-                // Same fixups as ExprKind::Call.
                 self.print_expr_maybe_paren(
                     expr,
                     parser::PREC_POSTFIX,
-                    FixupContext {
+                    fixup.leftmost_subexpression(),
-                        stmt: false,
-                        leftmost_subexpression_in_stmt: fixup.stmt
-                            || fixup.leftmost_subexpression_in_stmt,
-                        ..fixup
-                    },
                 );
                 self.word("[");
                 self.print_expr(index, FixupContext::default());
@@ -665,31 +588,14 @@ impl<'a> State<'a> {
                 // a "normal" binop gets parenthesized. (`LOr` is the lowest-precedence binop.)
                 let fake_prec = AssocOp::LOr.precedence() as i8;
                 if let Some(e) = start {
-                    self.print_expr_maybe_paren(
+                    self.print_expr_maybe_paren(e, fake_prec, fixup.leftmost_subexpression());
-                        e,
-                        fake_prec,
-                        FixupContext {
-                            stmt: false,
-                            leftmost_subexpression_in_stmt: fixup.stmt
-                                || fixup.leftmost_subexpression_in_stmt,
-                            ..fixup
-                        },
-                    );
                 }
                 match limits {
                     ast::RangeLimits::HalfOpen => self.word(".."),
                     ast::RangeLimits::Closed => self.word("..="),
                 }
                 if let Some(e) = end {
-                    self.print_expr_maybe_paren(
+                    self.print_expr_maybe_paren(e, fake_prec, fixup.subsequent_subexpression());
-                        e,
-                        fake_prec,
-                        FixupContext {
-                            stmt: false,
-                            leftmost_subexpression_in_stmt: false,
-                            ..fixup
-                        },
-                    );
                 }
             }
             ast::ExprKind::Underscore => self.word("_"),
@@ -706,11 +612,7 @@ impl<'a> State<'a> {
                     self.print_expr_maybe_paren(
                         expr,
                         parser::PREC_JUMP,
-                        FixupContext {
+                        fixup.subsequent_subexpression(),
-                            stmt: false,
-                            leftmost_subexpression_in_stmt: false,
-                            ..fixup
-                        },
                     );
                 }
             }
@@ -728,11 +630,7 @@ impl<'a> State<'a> {
                     self.print_expr_maybe_paren(
                         expr,
                         parser::PREC_JUMP,
-                        FixupContext {
+                        fixup.subsequent_subexpression(),
-                            stmt: false,
-                            leftmost_subexpression_in_stmt: false,
-                            ..fixup
-                        },
                     );
                 }
             }
@@ -745,11 +643,7 @@ impl<'a> State<'a> {
                     self.print_expr_maybe_paren(
                         expr,
                         parser::PREC_JUMP,
-                        FixupContext {
+                        fixup.subsequent_subexpression(),
-                            stmt: false,
-                            leftmost_subexpression_in_stmt: false,
-                            ..fixup
-                        },
                     );
                 }
             }
@@ -759,7 +653,7 @@ impl<'a> State<'a> {
                 self.print_expr_maybe_paren(
                     result,
                     parser::PREC_JUMP,
-                    FixupContext { stmt: false, leftmost_subexpression_in_stmt: false, ..fixup },
+                    fixup.subsequent_subexpression(),
                 );
             }
             ast::ExprKind::InlineAsm(a) => {
@@ -813,16 +707,11 @@ impl<'a> State<'a> {
                     self.print_expr_maybe_paren(
                         expr,
                         parser::PREC_JUMP,
-                        FixupContext {
+                        fixup.subsequent_subexpression(),
-                            stmt: false,
-                            leftmost_subexpression_in_stmt: false,
-                            ..fixup
-                        },
                     );
                 }
             }
             ast::ExprKind::Try(e) => {
-                // Same fixups as ExprKind::MethodCall.
                 self.print_expr_maybe_paren(e, parser::PREC_POSTFIX, fixup);
                 self.word("?")
             }
@@ -890,7 +779,7 @@ impl<'a> State<'a> {
                 }
                 _ => {
                     self.end(); // Close the ibox for the pattern.
-                    self.print_expr(body, FixupContext { stmt: true, ..FixupContext::default() });
+                    self.print_expr(body, FixupContext::new_stmt());
                     self.word(",");
                 }
             }

compiler/rustc_ast_pretty/src/pprust/state/fixup.rs

@@ -1,3 +1,6 @@
+use rustc_ast::util::{classify, parser};
+use rustc_ast::Expr;
+
 #[derive(Copy, Clone, Debug)]
 pub(crate) struct FixupContext {
     /// Print expression such that it can be parsed back as a statement
@@ -11,7 +14,7 @@ pub(crate) struct FixupContext {
     ///
     /// match x {};  // not when its own statement
     /// ```
-    pub stmt: bool,
+    stmt: bool,
 
     /// This is the difference between:
     ///
@@ -44,7 +47,7 @@ pub(crate) struct FixupContext {
     ///     Example: `$match;`
     ///
     ///     No parentheses required.
-    pub leftmost_subexpression_in_stmt: bool,
+    leftmost_subexpression_in_stmt: bool,
 
     /// This is the difference between:
     ///
@@ -55,7 +58,7 @@ pub(crate) struct FixupContext {
     ///     () if let _ = Struct {} => {}  // no parens
     /// }
     /// ```
-    pub parenthesize_exterior_struct_lit: bool,
+    parenthesize_exterior_struct_lit: bool,
 }
 
 /// The default amount of fixing is minimal fixing. Fixups should be turned on
@@ -69,3 +72,78 @@ 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 "condition"
+    /// of an `if` or `while`. There are a few other positions which are
+    /// grammatically equivalent and also use this, such as the iterator
+    /// expression in `for` and the scrutinee in `match`.
+    pub fn new_cond() -> Self {
+        FixupContext { parenthesize_exterior_struct_lit: true, ..FixupContext::default() }
+    }
+
+    /// Transform this fixup into the one that should apply when printing the
+    /// leftmost subexpression of the current expression.
+    ///
+    /// The leftmost subexpression is any subexpression that has the same first
+    /// token as the current expression, but has a different last token.
+    ///
+    /// For example in `$a + $b` and `$a.method()`, the subexpression `$a` is a
+    /// leftmost subexpression.
+    ///
+    /// Not every expression has a leftmost subexpression. For example neither
+    /// `-$a` nor `[$a]` have one.
+    pub fn leftmost_subexpression(self) -> Self {
+        FixupContext {
+            stmt: false,
+            leftmost_subexpression_in_stmt: self.stmt || self.leftmost_subexpression_in_stmt,
+            ..self
+        }
+    }
+
+    /// Transform this fixup into the one that should apply when printing any
+    /// subexpression that is neither a leftmost subexpression nor surrounded in
+    /// delimiters.
+    ///
+    /// 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 fn subsequent_subexpression(self) -> Self {
+        FixupContext { stmt: false, leftmost_subexpression_in_stmt: false, ..self }
+    }
+
+    /// Determine whether parentheses are needed around the given expression to
+    /// head off an unintended statement boundary.
+    ///
+    /// 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)
+    }
+
+    /// Determine whether parentheses are needed around the given `let`
+    /// scrutinee.
+    ///
+    /// In `if let _ = $e {}`, some examples of `$e` that would need parentheses
+    /// are:
+    ///
+    ///   - `Struct {}.f()`, because otherwise the `{` would be misinterpreted
+    ///     as the opening of the if's then-block.
+    ///
+    ///   - `true && false`, because otherwise this would be misinterpreted as a
+    ///     "let chain".
+    pub 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().order())
+    }
+}