Rollup merge of #124191 - dtolnay:fixup, r=compiler-errors

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

There are only 7 distinct ways that the AST pretty-printer interacts with FixupContext: 3 constructors (including Default), 2 transformations, and 2 queries.

This PR turns these into associated functions which can be documented with examples.

This PR unblocks https://github.com/rust-lang/rust/pull/119427#discussion_r1439481201. In order to improve the pretty-printer's behavior regarding parenthesization of braced macro calls in match arms, which have different grammar than macro calls in statements, FixupContext needs to be extended with 2 new fields. In the previous approach, that would be onerous. In the new approach, all it entails is 1 new constructor (`FixupContext::new_match_arm()`).
This commit is contained in:
许杰友 Jieyou Xu (Joe) 2024-04-20 21:45:38 +01:00 committed by GitHub
commit b3f9117a2d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 178 additions and 224 deletions

View File

@ -3,11 +3,12 @@
//! Note that HIR pretty printing is layered on top of this crate.
mod expr;
mod fixup;
mod item;
use crate::pp::Breaks::{Consistent, Inconsistent};
use crate::pp::{self, Breaks};
use crate::pprust::state::expr::FixupContext;
use crate::pprust::state::fixup::FixupContext;
use ast::TraitBoundModifiers;
use rustc_ast::attr::AttrIdGenerator;
use rustc_ast::ptr::P;
@ -15,7 +16,6 @@
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};
@ -1252,22 +1252,14 @@ fn print_stmt(&mut self, st: &ast::Stmt) {
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 => {
@ -1319,11 +1311,7 @@ fn print_block_maybe_unclosed(
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),
@ -1367,8 +1355,7 @@ fn print_let(&mut self, pat: &ast::Pat, expr: &ast::Expr, fixup: FixupContext) {
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(),
);
}

View File

@ -1,10 +1,10 @@
use crate::pp::Breaks::Inconsistent;
use crate::pprust::state::fixup::FixupContext;
use crate::pprust::state::{AnnNode, PrintState, State, INDENT_UNIT};
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};
@ -14,78 +14,6 @@
};
use std::fmt::Write;
#[derive(Copy, Clone, Debug)]
pub(crate) struct FixupContext {
/// Print expression such that it can be parsed back as a statement
/// consisting of the original expression.
///
/// The effect of this is for binary operators in statement position to set
/// `leftmost_subexpression_in_stmt` when printing their left-hand operand.
///
/// ```ignore (illustrative)
/// (match x {}) - 1; // match needs parens when LHS of binary operator
///
/// match x {}; // not when its own statement
/// ```
pub stmt: bool,
/// This is the difference between:
///
/// ```ignore (illustrative)
/// (match x {}) - 1; // subexpression needs parens
///
/// let _ = match x {} - 1; // no parens
/// ```
///
/// There are 3 distinguishable contexts in which `print_expr` might be
/// called with the expression `$match` as its argument, where `$match`
/// represents an expression of kind `ExprKind::Match`:
///
/// - stmt=false leftmost_subexpression_in_stmt=false
///
/// Example: `let _ = $match - 1;`
///
/// No parentheses required.
///
/// - stmt=false leftmost_subexpression_in_stmt=true
///
/// Example: `$match - 1;`
///
/// Must parenthesize `($match)`, otherwise parsing back the output as a
/// statement would terminate the statement after the closing brace of
/// the match, parsing `-1;` as a separate statement.
///
/// - stmt=true leftmost_subexpression_in_stmt=false
///
/// Example: `$match;`
///
/// No parentheses required.
pub leftmost_subexpression_in_stmt: bool,
/// This is the difference between:
///
/// ```ignore (illustrative)
/// if let _ = (Struct {}) {} // needs parens
///
/// match () {
/// () if let _ = Struct {} => {} // no parens
/// }
/// ```
pub parenthesize_exterior_struct_lit: bool,
}
/// The default amount of fixing is minimal fixing. Fixups should be turned on
/// in a targeted fashion where needed.
impl Default for FixupContext {
fn default() -> Self {
FixupContext {
stmt: false,
leftmost_subexpression_in_stmt: false,
parenthesize_exterior_struct_lit: false,
}
}
}
impl<'a> State<'a> {
fn print_else(&mut self, els: Option<&ast::Expr>) {
if let Some(_else) = els {
@ -136,9 +64,7 @@ fn print_expr_maybe_paren(&mut self, expr: &ast::Expr, prec: i8, fixup: FixupCon
/// 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?
@ -310,15 +236,7 @@ fn print_expr_call(&mut self, func: &ast::Expr, args: &[P<ast::Expr>], fixup: Fi
// 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)
}
@ -387,33 +305,17 @@ fn print_expr_binary(
_ => 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(
@ -431,11 +333,7 @@ fn print_expr_addr_of(
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) {
@ -470,8 +368,7 @@ pub(super) fn print_expr_outer_attr_style(
//
// 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();
@ -519,16 +416,7 @@ pub(super) fn print_expr_outer_attr_style(
}
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);
@ -660,70 +548,34 @@ pub(super) fn print_expr_outer_attr_style(
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());
@ -736,31 +588,14 @@ pub(super) fn print_expr_outer_attr_style(
// 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("_"),
@ -777,11 +612,7 @@ pub(super) fn print_expr_outer_attr_style(
self.print_expr_maybe_paren(
expr,
parser::PREC_JUMP,
FixupContext {
stmt: false,
leftmost_subexpression_in_stmt: false,
..fixup
},
fixup.subsequent_subexpression(),
);
}
}
@ -799,11 +630,7 @@ pub(super) fn print_expr_outer_attr_style(
self.print_expr_maybe_paren(
expr,
parser::PREC_JUMP,
FixupContext {
stmt: false,
leftmost_subexpression_in_stmt: false,
..fixup
},
fixup.subsequent_subexpression(),
);
}
}
@ -816,11 +643,7 @@ pub(super) fn print_expr_outer_attr_style(
self.print_expr_maybe_paren(
expr,
parser::PREC_JUMP,
FixupContext {
stmt: false,
leftmost_subexpression_in_stmt: false,
..fixup
},
fixup.subsequent_subexpression(),
);
}
}
@ -830,7 +653,7 @@ pub(super) fn print_expr_outer_attr_style(
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) => {
@ -884,16 +707,11 @@ pub(super) fn print_expr_outer_attr_style(
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("?")
}
@ -961,7 +779,7 @@ fn print_arm(&mut self, arm: &ast::Arm) {
}
_ => {
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(",");
}
}

View File

@ -0,0 +1,149 @@
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
/// consisting of the original expression.
///
/// The effect of this is for binary operators in statement position to set
/// `leftmost_subexpression_in_stmt` when printing their left-hand operand.
///
/// ```ignore (illustrative)
/// (match x {}) - 1; // match needs parens when LHS of binary operator
///
/// match x {}; // not when its own statement
/// ```
stmt: bool,
/// This is the difference between:
///
/// ```ignore (illustrative)
/// (match x {}) - 1; // subexpression needs parens
///
/// let _ = match x {} - 1; // no parens
/// ```
///
/// There are 3 distinguishable contexts in which `print_expr` might be
/// called with the expression `$match` as its argument, where `$match`
/// represents an expression of kind `ExprKind::Match`:
///
/// - stmt=false leftmost_subexpression_in_stmt=false
///
/// Example: `let _ = $match - 1;`
///
/// No parentheses required.
///
/// - stmt=false leftmost_subexpression_in_stmt=true
///
/// Example: `$match - 1;`
///
/// Must parenthesize `($match)`, otherwise parsing back the output as a
/// statement would terminate the statement after the closing brace of
/// the match, parsing `-1;` as a separate statement.
///
/// - stmt=true leftmost_subexpression_in_stmt=false
///
/// Example: `$match;`
///
/// No parentheses required.
leftmost_subexpression_in_stmt: bool,
/// This is the difference between:
///
/// ```ignore (illustrative)
/// if let _ = (Struct {}) {} // needs parens
///
/// match () {
/// () if let _ = Struct {} => {} // no parens
/// }
/// ```
parenthesize_exterior_struct_lit: bool,
}
/// The default amount of fixing is minimal fixing. Fixups should be turned on
/// in a targeted fashion where needed.
impl Default for FixupContext {
fn default() -> Self {
FixupContext {
stmt: false,
leftmost_subexpression_in_stmt: false,
parenthesize_exterior_struct_lit: false,
}
}
}
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())
}
}

View File

@ -1,5 +1,5 @@
use crate::pp::Breaks::Inconsistent;
use crate::pprust::state::expr::FixupContext;
use crate::pprust::state::fixup::FixupContext;
use crate::pprust::state::{AnnNode, PrintState, State, INDENT_UNIT};
use ast::StaticItem;