Skip to content

Do not parenthesize exterior struct lit inside match guards #118726

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Dec 11, 2023
Merged

Do not parenthesize exterior struct lit inside match guards #118726

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
419b269
Add if_let_guard and let_chains pretty printer tests
dtolnay Nov 30, 2023
d2b7bd4
Inline npals closure
dtolnay Nov 30, 2023
8d64961
Inline cond_needs_par into print_let
dtolnay Nov 30, 2023
dc5ec72
Rearrange logic of needs_par computation in print_let
dtolnay Nov 30, 2023
7f314ac
Delete special handling of some expr kinds from print_let
dtolnay Nov 30, 2023
6f1d763
Do not parenthesize exterior struct lit inside match guards
dtolnay Nov 30, 2023
8997215
Fix tidy ```ignore error
dtolnay Dec 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 51 additions & 22 deletions compiler/rustc_ast_pretty/src/pprust/state.rs
View file
Open in desktop
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 @@ -798,7 +799,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 @@ -903,7 +904,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 @@ -940,7 +941,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 @@ -1007,12 +1008,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 @@ -1068,7 +1069,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 @@ -1082,14 +1083,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 @@ -1141,7 +1142,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 @@ -1154,13 +1155,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 @@ -1207,7 +1236,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 @@ -1216,7 +1245,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 @@ -1226,26 +1255,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 @@ -1439,18 +1468,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 @@ -1604,7 +1633,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