Skip to content

Commit

Permalink
Unrolled build for rust-lang#118726
Browse files Browse the repository at this point in the history 
Rollup merge of rust-lang#118726 - dtolnay:matchguardlet, r=compiler-errors

Do not parenthesize exterior struct lit inside match guards

Before this PR, the AST pretty-printer injects parentheses around expressions any time parens _could_ be needed depending on what else is in the code that surrounds that expression. But the pretty-printer did not pass around enough context to understand whether parentheses really _are_ needed on any particular expression. As a consequence, there are false positives where unneeded parentheses are being inserted.

Example:

```rust
#![feature(if_let_guard)]

macro_rules! pp {
    ($e:expr) => {
        stringify!($e)
    };
}

fn main() {
    println!("{}", pp!(match () { () if let _ = Struct {} => {} }));
}
```

**Before:**

```console
match () { () if let _ = (Struct {}) => {} }
```

**After:**

```console
match () { () if let _ = Struct {} => {} }
```

This PR introduces a bit of state that is passed across various expression printing methods to help understand accurately whether particular situations require parentheses injected by the pretty printer, and it fixes one such false positive involving match guards as shown above.

There are other parenthesization false positive cases not fixed by this PR. I intend to address these in follow-up PRs. For example here is one: the expression `{ let _ = match x {} + 1; }` is pretty-printed as `{ let _ = (match x {}) + 1; }` despite there being no reason for parentheses to appear there.
  • Loading branch information
@rust-timer
rust-timer authored Dec 11, 2023
2 parents 5701093 + 8997215 commit 7a5899e
Show file tree
Hide file tree
Showing 4 changed files with 174 additions and 77 deletions.
73 changes: 51 additions & 22 deletions compiler/rustc_ast_pretty/src/pprust/state.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ mod item;

use crate::pp::Breaks::{Consistent, Inconsistent};
use crate::pp::{self, Breaks};
use crate::pprust::state::expr::FixupContext;
use rustc_ast::attr::AttrIdGenerator;
use rustc_ast::ptr::P;
use rustc_ast::token::{self, BinOpToken, CommentKind, Delimiter, Nonterminal, Token, TokenKind};
Expand Down Expand Up @@ -811,7 +812,7 @@ pub trait PrintState<'a>: std::ops::Deref<Target = pp::Printer> + std::ops::Dere
}

fn expr_to_string(&self, e: &ast::Expr) -> String {
Self::to_string(|s| s.print_expr(e))
Self::to_string(|s| s.print_expr(e, FixupContext::default()))
}

fn meta_item_lit_to_string(&self, lit: &ast::MetaItemLit) -> String {
Expand Down Expand Up @@ -916,7 +917,7 @@ impl<'a> State<'a> {
}

fn commasep_exprs(&mut self, b: Breaks, exprs: &[P<ast::Expr>]) {
self.commasep_cmnt(b, exprs, |s, e| s.print_expr(e), |e| e.span)
self.commasep_cmnt(b, exprs, |s, e| s.print_expr(e, FixupContext::default()), |e| e.span)
}

pub fn print_opt_lifetime(&mut self, lifetime: &Option<ast::Lifetime>) {
Expand Down Expand Up @@ -953,7 +954,7 @@ impl<'a> State<'a> {
match generic_arg {
GenericArg::Lifetime(lt) => self.print_lifetime(*lt),
GenericArg::Type(ty) => self.print_type(ty),
GenericArg::Const(ct) => self.print_expr(&ct.value),
GenericArg::Const(ct) => self.print_expr(&ct.value, FixupContext::default()),
}
}

Expand Down Expand Up @@ -1020,12 +1021,12 @@ impl<'a> State<'a> {
self.word("[");
self.print_type(ty);
self.word("; ");
self.print_expr(&length.value);
self.print_expr(&length.value, FixupContext::default());
self.word("]");
}
ast::TyKind::Typeof(e) => {
self.word("typeof(");
self.print_expr(&e.value);
self.print_expr(&e.value, FixupContext::default());
self.word(")");
}
ast::TyKind::Infer => {
Expand Down Expand Up @@ -1081,7 +1082,7 @@ impl<'a> State<'a> {
if let Some((init, els)) = loc.kind.init_else_opt() {
self.nbsp();
self.word_space("=");
self.print_expr(init);
self.print_expr(init, FixupContext::default());
if let Some(els) = els {
self.cbox(INDENT_UNIT);
self.ibox(INDENT_UNIT);
Expand All @@ -1095,14 +1096,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);
self.print_expr_outer_attr_style(expr, false, 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);
self.print_expr_outer_attr_style(expr, false, FixupContext::default());
self.word(";");
}
ast::StmtKind::Empty => {
Expand Down Expand Up @@ -1154,7 +1155,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);
self.print_expr_outer_attr_style(expr, false, FixupContext::default());
self.maybe_print_trailing_comment(expr.span, Some(blk.span.hi()));
}
_ => self.print_stmt(st),
Expand All @@ -1167,13 +1168,41 @@ impl<'a> State<'a> {
}

/// Print a `let pat = expr` expression.
fn print_let(&mut self, pat: &ast::Pat, expr: &ast::Expr) {
///
/// Parentheses are inserted surrounding `expr` if a round-trip through the
/// parser would otherwise work out the wrong way in a condition position.
///
/// For example each of the following would mean the wrong thing without
/// parentheses.
///
/// ```ignore (illustrative)
/// if let _ = (Struct {}) {}
///
/// if let _ = (true && false) {}
/// ```
///
/// In a match guard, the second case still requires parens, but the first
/// case no longer does because anything until `=>` is considered part of
/// the match guard expression. Parsing of the expression is not terminated
/// by `{` in that position.
///
/// ```ignore (illustrative)
/// match () {
/// () if let _ = Struct {} => {}
/// () if let _ = (true && false) => {}
/// }
/// ```
fn print_let(&mut self, pat: &ast::Pat, expr: &ast::Expr, fixup: FixupContext) {
self.word("let ");
self.print_pat(pat);
self.space();
self.word_space("=");
let npals = || parser::needs_par_as_let_scrutinee(expr.precedence().order());
self.print_expr_cond_paren(expr, Self::cond_needs_par(expr) || npals())
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()),
FixupContext::default(),
);
}

fn print_mac(&mut self, m: &ast::MacCall) {
Expand Down Expand Up @@ -1220,7 +1249,7 @@ impl<'a> State<'a> {
print_reg_or_class(s, reg);
s.pclose();
s.space();
s.print_expr(expr);
s.print_expr(expr, FixupContext::default());
}
InlineAsmOperand::Out { reg, late, expr } => {
s.word(if *late { "lateout" } else { "out" });
Expand All @@ -1229,7 +1258,7 @@ impl<'a> State<'a> {
s.pclose();
s.space();
match expr {
Some(expr) => s.print_expr(expr),
Some(expr) => s.print_expr(expr, FixupContext::default()),
None => s.word("_"),
}
}
Expand All @@ -1239,26 +1268,26 @@ impl<'a> State<'a> {
print_reg_or_class(s, reg);
s.pclose();
s.space();
s.print_expr(expr);
s.print_expr(expr, FixupContext::default());
}
InlineAsmOperand::SplitInOut { reg, late, in_expr, out_expr } => {
s.word(if *late { "inlateout" } else { "inout" });
s.popen();
print_reg_or_class(s, reg);
s.pclose();
s.space();
s.print_expr(in_expr);
s.print_expr(in_expr, FixupContext::default());
s.space();
s.word_space("=>");
match out_expr {
Some(out_expr) => s.print_expr(out_expr),
Some(out_expr) => s.print_expr(out_expr, FixupContext::default()),
None => s.word("_"),
}
}
InlineAsmOperand::Const { anon_const } => {
s.word("const");
s.space();
s.print_expr(&anon_const.value);
s.print_expr(&anon_const.value, FixupContext::default());
}
InlineAsmOperand::Sym { sym } => {
s.word("sym");
Expand Down Expand Up @@ -1452,18 +1481,18 @@ impl<'a> State<'a> {
self.print_pat(inner);
}
}
PatKind::Lit(e) => self.print_expr(e),
PatKind::Lit(e) => self.print_expr(e, FixupContext::default()),
PatKind::Range(begin, end, Spanned { node: end_kind, .. }) => {
if let Some(e) = begin {
self.print_expr(e);
self.print_expr(e, FixupContext::default());
}
match end_kind {
RangeEnd::Included(RangeSyntax::DotDotDot) => self.word("..."),
RangeEnd::Included(RangeSyntax::DotDotEq) => self.word("..="),
RangeEnd::Excluded => self.word(".."),
}
if let Some(e) = end {
self.print_expr(e);
self.print_expr(e, FixupContext::default());
}
}
PatKind::Slice(elts) => {
Expand Down Expand Up @@ -1617,7 +1646,7 @@ impl<'a> State<'a> {
if let Some(default) = default {
s.space();
s.word_space("=");
s.print_expr(&default.value);
s.print_expr(&default.value, FixupContext::default());
}
}
}
Expand Down
Loading

0 comments on commit 7a5899e

Please sign in to comment.