Skip to content

Commit

Permalink
Rewrite condition printing to parenthesize leafs instead of whole cond
Browse files Browse the repository at this point in the history
  • Loading branch information
dtolnay committed Jan 5, 2025
1 parent 52699de commit 3f8bac6
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 205 deletions.
139 changes: 0 additions & 139 deletions src/classify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,145 +67,6 @@ pub(crate) fn requires_comma_to_be_match_arm(expr: &Expr) -> bool {
}
}

#[cfg(all(feature = "printing", feature = "full"))]
pub(crate) fn confusable_with_adjacent_block(expr: &Expr) -> bool {
let allow_struct = false;
let optional_leftmost_subexpression = false;
let rightmost_subexpression = true;
return confusable(
expr,
allow_struct,
optional_leftmost_subexpression,
rightmost_subexpression,
);

fn confusable(
expr: &Expr,
allow_struct: bool,
optional_leftmost_subexpression: bool,
rightmost_subexpression: bool,
) -> bool {
match expr {
Expr::Assign(e) => {
confusable(
&e.left,
allow_struct,
optional_leftmost_subexpression,
false,
) || confusable(&e.right, allow_struct, false, rightmost_subexpression)
}
Expr::Await(e) => confusable(
&e.base,
allow_struct,
optional_leftmost_subexpression,
false,
),
Expr::Binary(e) => {
confusable(
&e.left,
allow_struct,
optional_leftmost_subexpression,
false,
) || confusable(&e.right, allow_struct, false, rightmost_subexpression)
}
Expr::Block(e) => {
optional_leftmost_subexpression && e.attrs.is_empty() && e.label.is_none()
}
Expr::Break(e) => {
if let Some(value) = &e.expr {
confusable(value, true, !allow_struct, rightmost_subexpression)
} else {
allow_struct && rightmost_subexpression
}
}
Expr::Call(e) => confusable(
&e.func,
allow_struct,
optional_leftmost_subexpression,
false,
),
Expr::Cast(e) => confusable(
&e.expr,
allow_struct,
optional_leftmost_subexpression,
false,
),
Expr::Closure(e) => confusable(&e.body, allow_struct, false, rightmost_subexpression),
Expr::Field(e) => confusable(
&e.base,
allow_struct,
optional_leftmost_subexpression,
false,
),
Expr::Index(e) => confusable(
&e.expr,
allow_struct,
optional_leftmost_subexpression,
false,
),
Expr::MethodCall(e) => confusable(
&e.receiver,
allow_struct,
optional_leftmost_subexpression,
false,
),
Expr::Path(_) => allow_struct && rightmost_subexpression,
Expr::Range(e) => {
(match &e.start {
Some(start) => {
confusable(start, allow_struct, optional_leftmost_subexpression, false)
}
None => false,
} || match &e.end {
Some(end) => {
confusable(end, allow_struct, !allow_struct, rightmost_subexpression)
}
None => allow_struct && rightmost_subexpression,
})
}
Expr::RawAddr(e) => confusable(&e.expr, allow_struct, false, rightmost_subexpression),
Expr::Reference(e) => confusable(&e.expr, allow_struct, false, rightmost_subexpression),
Expr::Return(e) => match &e.expr {
Some(expr) => confusable(expr, true, false, rightmost_subexpression),
None => rightmost_subexpression,
},
Expr::Struct(_) => !allow_struct,
Expr::Try(e) => confusable(
&e.expr,
allow_struct,
optional_leftmost_subexpression,
false,
),
Expr::Unary(e) => confusable(&e.expr, allow_struct, false, rightmost_subexpression),
Expr::Yield(e) => match &e.expr {
Some(expr) => confusable(expr, true, false, rightmost_subexpression),
None => rightmost_subexpression,
},

Expr::Array(_)
| Expr::Async(_)
| Expr::Const(_)
| Expr::Continue(_)
| Expr::ForLoop(_)
| Expr::Group(_)
| Expr::If(_)
| Expr::Infer(_)
| Expr::Let(_)
| Expr::Lit(_)
| Expr::Loop(_)
| Expr::Macro(_)
| Expr::Match(_)
| Expr::Paren(_)
| Expr::Repeat(_)
| Expr::TryBlock(_)
| Expr::Tuple(_)
| Expr::Unsafe(_)
| Expr::Verbatim(_)
| Expr::While(_) => false,
}
}
}

#[cfg(feature = "printing")]
pub(crate) fn trailing_unparameterized_path(mut ty: &Type) -> bool {
loop {
Expand Down
39 changes: 13 additions & 26 deletions src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3151,16 +3151,6 @@ pub(crate) mod printing {
#[cfg(not(feature = "full"))]
pub(crate) fn outer_attrs_to_tokens(_attrs: &[Attribute], _tokens: &mut TokenStream) {}

#[cfg(feature = "full")]
fn print_condition(expr: &Expr, tokens: &mut TokenStream) {
print_subexpression(
expr,
classify::confusable_with_adjacent_block(expr),
tokens,
FixupContext::new_condition(),
);
}

pub(crate) fn print_subexpression(
expr: &Expr,
needs_group: bool,
Expand Down Expand Up @@ -3193,7 +3183,7 @@ pub(crate) mod printing {

pub(crate) fn print_expr(expr: &Expr, tokens: &mut TokenStream, mut fixup: FixupContext) {
#[cfg(feature = "full")]
let needs_group = fixup.would_cause_statement_boundary(expr);
let needs_group = fixup.parenthesize(expr);
#[cfg(not(feature = "full"))]
let needs_group = false;

Expand Down Expand Up @@ -3430,7 +3420,7 @@ pub(crate) mod printing {
// ^---------------------------------^
e.label.is_none() && classify::expr_leading_label(value),
tokens,
fixup.rightmost_subexpression_fixup(Precedence::Jump),
fixup.rightmost_subexpression_fixup(true, true, Precedence::Jump),
);
}
}
Expand Down Expand Up @@ -3513,7 +3503,7 @@ pub(crate) mod printing {
print_expr(
&e.body,
tokens,
fixup.rightmost_subexpression_fixup(Precedence::Jump),
fixup.rightmost_subexpression_fixup(false, false, Precedence::Jump),
);
} else {
token::Brace::default().surround(tokens, |tokens| {
Expand Down Expand Up @@ -3574,7 +3564,7 @@ pub(crate) mod printing {
self.for_token.to_tokens(tokens);
self.pat.to_tokens(tokens);
self.in_token.to_tokens(tokens);
print_condition(&self.expr, tokens);
print_expr(&self.expr, tokens, FixupContext::new_condition());
self.body.brace_token.surround(tokens, |tokens| {
inner_attrs_to_tokens(&self.attrs, tokens);
tokens.append_all(&self.body.stmts);
Expand All @@ -3601,7 +3591,7 @@ pub(crate) mod printing {
let mut expr = self;
loop {
expr.if_token.to_tokens(tokens);
print_condition(&expr.cond, tokens);
print_expr(&expr.cond, tokens, FixupContext::new_condition());
expr.then_branch.to_tokens(tokens);

let (else_token, else_) = match &expr.else_branch {
Expand Down Expand Up @@ -3682,12 +3672,8 @@ pub(crate) mod printing {
e.let_token.to_tokens(tokens);
e.pat.to_tokens(tokens);
e.eq_token.to_tokens(tokens);
print_subexpression(
&e.expr,
fixup.needs_group_as_let_scrutinee(&e.expr),
tokens,
FixupContext::NONE,
);
let (right_prec, right_fixup) = fixup.rightmost_subexpression(&e.expr, Precedence::Let);
print_subexpression(&e.expr, right_prec < Precedence::Let, tokens, right_fixup);
}

#[cfg_attr(docsrs, doc(cfg(feature = "printing")))]
Expand Down Expand Up @@ -3726,7 +3712,7 @@ pub(crate) mod printing {
fn to_tokens(&self, tokens: &mut TokenStream) {
outer_attrs_to_tokens(&self.attrs, tokens);
self.match_token.to_tokens(tokens);
print_condition(&self.expr, tokens);
print_expr(&self.expr, tokens, FixupContext::new_condition());
self.brace_token.surround(tokens, |tokens| {
inner_attrs_to_tokens(&self.attrs, tokens);
for (i, arm) in self.arms.iter().enumerate() {
Expand Down Expand Up @@ -3811,7 +3797,8 @@ pub(crate) mod printing {
}
e.limits.to_tokens(tokens);
if let Some(end) = &e.end {
let (right_prec, right_fixup) = fixup.rightmost_subexpression(end, Precedence::Range);
let right_fixup = fixup.rightmost_subexpression_fixup(false, true, Precedence::Range);
let right_prec = right_fixup.rightmost_subexpression_precedence(end);
print_subexpression(end, right_prec <= Precedence::Range, tokens, right_fixup);
}
}
Expand Down Expand Up @@ -3892,7 +3879,7 @@ pub(crate) mod printing {
print_expr(
expr,
tokens,
fixup.rightmost_subexpression_fixup(Precedence::Jump),
fixup.rightmost_subexpression_fixup(true, false, Precedence::Jump),
);
}
}
Expand Down Expand Up @@ -4003,7 +3990,7 @@ pub(crate) mod printing {
outer_attrs_to_tokens(&self.attrs, tokens);
self.label.to_tokens(tokens);
self.while_token.to_tokens(tokens);
print_condition(&self.cond, tokens);
print_expr(&self.cond, tokens, FixupContext::new_condition());
self.body.brace_token.surround(tokens, |tokens| {
inner_attrs_to_tokens(&self.attrs, tokens);
tokens.append_all(&self.body.stmts);
Expand All @@ -4027,7 +4014,7 @@ pub(crate) mod printing {
print_expr(
expr,
tokens,
fixup.rightmost_subexpression_fixup(Precedence::Jump),
fixup.rightmost_subexpression_fixup(true, false, Precedence::Jump),
);
}
}
Expand Down
Loading

0 comments on commit 3f8bac6

Please sign in to comment.