diff options
| -rw-r--r-- | compiler/rustc_ast_pretty/src/pprust/state.rs | 18 | ||||
| -rw-r--r-- | compiler/rustc_ast_pretty/src/pprust/state/expr.rs | 324 | ||||
| -rw-r--r-- | tests/ui/macros/stringify.rs | 18 |
3 files changed, 304 insertions, 56 deletions
diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index d6c15ec35b6..1ca4b88c840 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -1096,14 +1096,22 @@ 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::default()); + self.print_expr_outer_attr_style( + 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(expr, false, FixupContext::default()); + self.print_expr_outer_attr_style( + expr, + false, + FixupContext { stmt: true, ..FixupContext::default() }, + ); self.word(";"); } ast::StmtKind::Empty => { @@ -1155,7 +1163,11 @@ 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::default()); + self.print_expr_outer_attr_style( + expr, + false, + FixupContext { stmt: true, ..FixupContext::default() }, + ); self.maybe_print_trailing_comment(expr.span, Some(blk.span.hi())); } _ => self.print_stmt(st), diff --git a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs index f5ffcddb83d..4bd03006edf 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state/expr.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state/expr.rs @@ -3,6 +3,7 @@ use crate::pprust::state::{AnnNode, PrintState, State, INDENT_UNIT}; 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,6 +15,61 @@ 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, } @@ -21,7 +77,11 @@ pub(crate) struct FixupContext { /// in a targetted fashion where needed. impl Default for FixupContext { fn default() -> Self { - FixupContext { parenthesize_exterior_struct_lit: false } + FixupContext { + stmt: false, + leftmost_subexpression_in_stmt: false, + parenthesize_exterior_struct_lit: false, + } } } @@ -75,7 +135,8 @@ 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 }; + let fixup = + FixupContext { parenthesize_exterior_struct_lit: true, ..FixupContext::default() }; self.print_expr_cond_paren(expr, Self::cond_needs_par(expr), fixup) } @@ -98,26 +159,25 @@ impl<'a> State<'a> { &mut self, expr: &ast::Expr, needs_par: bool, - fixup: FixupContext, + mut fixup: FixupContext, ) { if needs_par { self.popen(); + + // If we are surrounding the whole cond in parentheses, such as: + // + // if (return Struct {}) {} + // + // then there is no need for parenthesizing the individual struct + // expressions within. On the other hand if the whole cond is not + // parenthesized, then print_expr must parenthesize exterior struct + // literals. + // + // if x == (Struct {}) {} + // + fixup = FixupContext::default(); } - // If we are surrounding the whole cond in parentheses, such as: - // - // if (return Struct {}) {} - // - // then there is no need for parenthesizing the individual struct - // expressions within. On the other hand if the whole cond is not - // parenthesized, then print_expr must parenthesize exterior struct - // literals. - // - // if x == (Struct {}) {} - // - let fixup = FixupContext { - parenthesize_exterior_struct_lit: fixup.parenthesize_exterior_struct_lit && !needs_par, - }; self.print_expr(expr, fixup); if needs_par { @@ -233,7 +293,32 @@ impl<'a> State<'a> { _ => parser::PREC_POSTFIX, }; - self.print_expr_maybe_paren(func, prec, fixup); + // Independent of parenthesization related to precedence, we must + // parenthesize `func` if this is a statement context in which without + // parentheses, a statement boundary would occur inside `func` or + // immediately after `func`. + // + // Suppose `func` represents `match () { _ => f }`. We must produce: + // + // (match () { _ => f })(); + // + // instead of: + // + // match () { _ => f } (); + // + // 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_call_post(args) } @@ -244,7 +329,17 @@ impl<'a> State<'a> { base_args: &[P<ast::Expr>], fixup: FixupContext, ) { + // Unlike in `print_expr_call`, no change to fixup here because + // statement boundaries never occur in front of a `.` (or `?`) token. + // + // match () { _ => f }.method(); + // + // Parenthesizing only for precedence and not with regard to statement + // boundaries, `$receiver.method()` can be parsed back as a statement + // containing an expression if and only if `$receiver` can be parsed as + // a statement containing an expression. self.print_expr_maybe_paren(receiver, parser::PREC_POSTFIX, fixup); + self.word("."); self.print_ident(segment.ident); if let Some(args) = &segment.args { @@ -288,22 +383,36 @@ impl<'a> State<'a> { (&ast::ExprKind::Let { .. }, _) if !parser::needs_par_as_let_scrutinee(prec) => { parser::PREC_FORCE_PAREN } - // For a binary expression like `(match () { _ => a }) OP b`, the parens are required - // otherwise the parser would interpret `match () { _ => a }` as a statement, - // with the remaining `OP b` not making sense. So we force parens. - (&ast::ExprKind::Match(..), _) => parser::PREC_FORCE_PAREN, _ => left_prec, }; - self.print_expr_maybe_paren(lhs, left_prec, fixup); + self.print_expr_maybe_paren( + 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(rhs, right_prec, fixup) + + self.print_expr_maybe_paren( + 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(expr, parser::PREC_PREFIX, fixup) + self.print_expr_maybe_paren( + expr, + parser::PREC_PREFIX, + FixupContext { stmt: false, leftmost_subexpression_in_stmt: false, ..fixup }, + ); } fn print_expr_addr_of( @@ -321,7 +430,11 @@ impl<'a> State<'a> { self.print_mutability(mutability, true); } } - self.print_expr_maybe_paren(expr, parser::PREC_PREFIX, fixup) + self.print_expr_maybe_paren( + 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) { @@ -332,7 +445,7 @@ impl<'a> State<'a> { &mut self, expr: &ast::Expr, is_inline: bool, - fixup: FixupContext, + mut fixup: FixupContext, ) { self.maybe_print_comment(expr.span.lo()); @@ -344,7 +457,27 @@ impl<'a> State<'a> { } self.ibox(INDENT_UNIT); + + // The Match subexpression in `match x {} - 1` must be parenthesized if + // it is the leftmost subexpression in a statement: + // + // (match x {}) - 1; + // + // But not otherwise: + // + // let _ = match x {} - 1; + // + // 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); + if needs_par { + self.popen(); + fixup = FixupContext::default(); + } + self.ann.pre(self, AnnNode::Expr(expr)); + match &expr.kind { ast::ExprKind::Array(exprs) => { self.print_expr_vec(exprs); @@ -385,7 +518,16 @@ impl<'a> State<'a> { } ast::ExprKind::Cast(expr, ty) => { let prec = AssocOp::As.precedence() as i8; - self.print_expr_maybe_paren(expr, prec, fixup); + self.print_expr_maybe_paren( + 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); @@ -504,31 +646,71 @@ 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, fixup); + self.print_expr_maybe_paren( + 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(rhs, prec, fixup); + self.print_expr_maybe_paren( + 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(lhs, prec + 1, fixup); + self.print_expr_maybe_paren( + 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(rhs, prec, fixup); + self.print_expr_maybe_paren( + 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, _) => { - self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX, fixup); + // 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 + }, + ); self.word("["); self.print_expr(index, FixupContext::default()); self.word("]"); @@ -540,14 +722,31 @@ 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, fixup); + self.print_expr_maybe_paren( + 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(e, fake_prec, fixup); + self.print_expr_maybe_paren( + e, + fake_prec, + FixupContext { + stmt: false, + leftmost_subexpression_in_stmt: false, + ..fixup + }, + ); } } ast::ExprKind::Underscore => self.word("_"), @@ -561,7 +760,15 @@ impl<'a> State<'a> { } if let Some(expr) = opt_expr { self.space(); - self.print_expr_maybe_paren(expr, parser::PREC_JUMP, fixup); + self.print_expr_maybe_paren( + expr, + parser::PREC_JUMP, + FixupContext { + stmt: false, + leftmost_subexpression_in_stmt: false, + ..fixup + }, + ); } } ast::ExprKind::Continue(opt_label) => { @@ -575,7 +782,15 @@ impl<'a> State<'a> { self.word("return"); if let Some(expr) = result { self.word(" "); - self.print_expr_maybe_paren(expr, parser::PREC_JUMP, fixup); + self.print_expr_maybe_paren( + expr, + parser::PREC_JUMP, + FixupContext { + stmt: false, + leftmost_subexpression_in_stmt: false, + ..fixup + }, + ); } } ast::ExprKind::Yeet(result) => { @@ -584,13 +799,25 @@ impl<'a> State<'a> { self.word("yeet"); if let Some(expr) = result { self.word(" "); - self.print_expr_maybe_paren(expr, parser::PREC_JUMP, fixup); + self.print_expr_maybe_paren( + expr, + parser::PREC_JUMP, + FixupContext { + stmt: false, + leftmost_subexpression_in_stmt: false, + ..fixup + }, + ); } } ast::ExprKind::Become(result) => { self.word("become"); self.word(" "); - self.print_expr_maybe_paren(result, parser::PREC_JUMP, fixup); + self.print_expr_maybe_paren( + result, + parser::PREC_JUMP, + FixupContext { stmt: false, leftmost_subexpression_in_stmt: false, ..fixup }, + ); } ast::ExprKind::InlineAsm(a) => { // FIXME: This should have its own syntax, distinct from a macro invocation. @@ -640,10 +867,19 @@ impl<'a> State<'a> { if let Some(expr) = e { self.space(); - self.print_expr_maybe_paren(expr, parser::PREC_JUMP, fixup); + self.print_expr_maybe_paren( + expr, + parser::PREC_JUMP, + FixupContext { + 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("?") } @@ -659,7 +895,13 @@ impl<'a> State<'a> { self.pclose() } } + self.ann.post(self, AnnNode::Expr(expr)); + + if needs_par { + self.pclose(); + } + self.end(); } @@ -700,7 +942,7 @@ impl<'a> State<'a> { } _ => { self.end(); // Close the ibox for the pattern. - self.print_expr(body, FixupContext::default()); + self.print_expr(body, FixupContext { stmt: true, ..FixupContext::default() }); self.word(","); } } diff --git a/tests/ui/macros/stringify.rs b/tests/ui/macros/stringify.rs index 6cd64e3f430..192e6e0cc98 100644 --- a/tests/ui/macros/stringify.rs +++ b/tests/ui/macros/stringify.rs @@ -211,8 +211,7 @@ fn test_expr() { } c2_match_arm!( [ { 1 } - 1 ], - // FIXME(dtolnay): this is invalid syntax, needs parens. - "match () { _ => { 1 } - 1, }", + "match () { _ => ({ 1 }) - 1, }", "match() { _ => { 1 } - 1 }", ); @@ -669,8 +668,7 @@ fn test_stmt() { } c2_let_expr_minus_one!( [ match void {} ], - // FIXME(dtolnay): no parens needed. - "let _ = (match void {}) - 1;", + "let _ = match void {} - 1;", "let _ = match void {} - 1", ); @@ -706,25 +704,21 @@ fn test_stmt() { c2_minus_one!( [ match void {} ], "(match void {}) - 1;", - // FIXME(dtolnay): no parens expected. - "(match void {}) - 1", + "match void {} - 1", ); c2_minus_one!( [ match void {}() ], - // FIXME(dtolnay): needs parens around match. - "match void {}() - 1;", + "(match void {})() - 1;", "match void {}() - 1", ); c2_minus_one!( [ match void {}[0] ], - // FIXME(dtolnay): needs parens around match. - "match void {}[0] - 1;", + "(match void {})[0] - 1;", "match void {}[0] - 1", ); c2_minus_one!( [ loop { break 1; } ], - // FIXME(dtolnay): needs parens around loop. - "loop { break 1; } - 1;", + "(loop { break 1; }) - 1;", "loop { break 1; } - 1", ); |