From debdb72ba8354db14a22103f43e9995ea8e2beb9 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Fri, 19 Apr 2024 22:22:14 -0700 Subject: [PATCH] Give a name to each distinct manipulation of pretty-printer FixupContext --- compiler/rustc_ast_pretty/src/pprust/state.rs | 22 +-- .../rustc_ast_pretty/src/pprust/state/expr.rs | 153 +++--------------- .../src/pprust/state/fixup.rs | 84 +++++++++- 3 files changed, 106 insertions(+), 153 deletions(-) diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index a310e1bf0710a..2c176828c841f 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/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( - expr, - false, - FixupContext { stmt: true, ..FixupContext::default() }, - ); + self.print_expr_outer_attr_style(expr, false, FixupContext::new_stmt()); 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( - expr, - false, - FixupContext { stmt: true, ..FixupContext::default() }, - ); + self.print_expr_outer_attr_style(expr, false, FixupContext::new_stmt()); 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( - expr, - false, - FixupContext { stmt: true, ..FixupContext::default() }, - ); + self.print_expr_outer_attr_style(expr, false, FixupContext::new_stmt()); 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) - || parser::needs_par_as_let_scrutinee(expr.precedence().order()), + fixup.needs_par_as_let_scrutinee(expr), FixupContext::default(), ); } diff --git a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs index 0aba0c092829a..4cbdc9f256dfe 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs +++ b/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 = - FixupContext { parenthesize_exterior_struct_lit: true, ..FixupContext::default() }; - self.print_expr_cond_paren(expr, Self::cond_needs_par(expr), fixup) + self.print_expr_cond_paren(expr, Self::cond_needs_par(expr), FixupContext::new_cond()) } /// 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( - func, - prec, - FixupContext { - stmt: false, - leftmost_subexpression_in_stmt: fixup.stmt || fixup.leftmost_subexpression_in_stmt, - ..fixup - }, - ); + self.print_expr_maybe_paren(func, prec, fixup.leftmost_subexpression()); self.print_call_post(args) } @@ -316,33 +305,17 @@ impl<'a> State<'a> { _ => left_prec, }; - self.print_expr_maybe_paren( - lhs, - left_prec, - FixupContext { - stmt: false, - leftmost_subexpression_in_stmt: fixup.stmt || fixup.leftmost_subexpression_in_stmt, - ..fixup - }, - ); + self.print_expr_maybe_paren(lhs, left_prec, fixup.leftmost_subexpression()); self.space(); self.word_space(op.node.as_str()); - self.print_expr_maybe_paren( - rhs, - right_prec, - FixupContext { stmt: false, leftmost_subexpression_in_stmt: false, ..fixup }, - ); + self.print_expr_maybe_paren(rhs, right_prec, fixup.subsequent_subexpression()); } fn print_expr_unary(&mut self, op: ast::UnOp, expr: &ast::Expr, fixup: FixupContext) { self.word(op.as_str()); - self.print_expr_maybe_paren( - expr, - parser::PREC_PREFIX, - FixupContext { stmt: false, leftmost_subexpression_in_stmt: false, ..fixup }, - ); + self.print_expr_maybe_paren(expr, parser::PREC_PREFIX, fixup.subsequent_subexpression()); } fn print_expr_addr_of( @@ -360,11 +333,7 @@ impl<'a> State<'a> { self.print_mutability(mutability, true); } } - self.print_expr_maybe_paren( - expr, - parser::PREC_PREFIX, - FixupContext { stmt: false, leftmost_subexpression_in_stmt: false, ..fixup }, - ); + self.print_expr_maybe_paren(expr, parser::PREC_PREFIX, fixup.subsequent_subexpression()); } 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 = - fixup.leftmost_subexpression_in_stmt && !classify::expr_requires_semi_to_be_stmt(expr); + let needs_par = fixup.would_cause_statement_boundary(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( - expr, - prec, - FixupContext { - stmt: false, - leftmost_subexpression_in_stmt: fixup.stmt - || fixup.leftmost_subexpression_in_stmt, - ..fixup - }, - ); + self.print_expr_maybe_paren(expr, prec, fixup.leftmost_subexpression()); 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( - lhs, - prec + 1, - FixupContext { - stmt: false, - leftmost_subexpression_in_stmt: fixup.stmt - || fixup.leftmost_subexpression_in_stmt, - ..fixup - }, - ); + self.print_expr_maybe_paren(lhs, prec + 1, fixup.leftmost_subexpression()); self.space(); self.word_space("="); - self.print_expr_maybe_paren( - rhs, - prec, - FixupContext { stmt: false, leftmost_subexpression_in_stmt: false, ..fixup }, - ); + self.print_expr_maybe_paren(rhs, prec, fixup.subsequent_subexpression()); } ast::ExprKind::AssignOp(op, lhs, rhs) => { - // Same fixups as ExprKind::Binary. let prec = AssocOp::Assign.precedence() as i8; - self.print_expr_maybe_paren( - lhs, - prec + 1, - FixupContext { - stmt: false, - leftmost_subexpression_in_stmt: fixup.stmt - || fixup.leftmost_subexpression_in_stmt, - ..fixup - }, - ); + self.print_expr_maybe_paren(lhs, prec + 1, fixup.leftmost_subexpression()); self.space(); self.word(op.node.as_str()); self.word_space("="); - self.print_expr_maybe_paren( - rhs, - prec, - FixupContext { stmt: false, leftmost_subexpression_in_stmt: false, ..fixup }, - ); + self.print_expr_maybe_paren(rhs, prec, fixup.subsequent_subexpression()); } 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 { - stmt: false, - leftmost_subexpression_in_stmt: fixup.stmt - || fixup.leftmost_subexpression_in_stmt, - ..fixup - }, + fixup.leftmost_subexpression(), ); 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( - e, - fake_prec, - FixupContext { - stmt: false, - leftmost_subexpression_in_stmt: fixup.stmt - || fixup.leftmost_subexpression_in_stmt, - ..fixup - }, - ); + self.print_expr_maybe_paren(e, fake_prec, fixup.leftmost_subexpression()); } match limits { ast::RangeLimits::HalfOpen => self.word(".."), ast::RangeLimits::Closed => self.word("..="), } if let Some(e) = end { - self.print_expr_maybe_paren( - e, - fake_prec, - FixupContext { - stmt: false, - leftmost_subexpression_in_stmt: false, - ..fixup - }, - ); + self.print_expr_maybe_paren(e, fake_prec, fixup.subsequent_subexpression()); } } ast::ExprKind::Underscore => self.word("_"), @@ -706,11 +612,7 @@ impl<'a> State<'a> { self.print_expr_maybe_paren( expr, parser::PREC_JUMP, - FixupContext { - stmt: false, - leftmost_subexpression_in_stmt: false, - ..fixup - }, + fixup.subsequent_subexpression(), ); } } @@ -728,11 +630,7 @@ impl<'a> State<'a> { self.print_expr_maybe_paren( expr, parser::PREC_JUMP, - FixupContext { - stmt: false, - leftmost_subexpression_in_stmt: false, - ..fixup - }, + fixup.subsequent_subexpression(), ); } } @@ -745,11 +643,7 @@ impl<'a> State<'a> { self.print_expr_maybe_paren( expr, parser::PREC_JUMP, - FixupContext { - stmt: false, - leftmost_subexpression_in_stmt: false, - ..fixup - }, + fixup.subsequent_subexpression(), ); } } @@ -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 { - stmt: false, - leftmost_subexpression_in_stmt: false, - ..fixup - }, + fixup.subsequent_subexpression(), ); } } 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(","); } } diff --git a/compiler/rustc_ast_pretty/src/pprust/state/fixup.rs b/compiler/rustc_ast_pretty/src/pprust/state/fixup.rs index ab46ebd0aab11..d21cb82f83b28 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state/fixup.rs +++ b/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()) + } +}