Skip to content

Commit

Permalink
Incorporate parenthesization of casts into FixupContext
Browse files Browse the repository at this point in the history
  • Loading branch information
@dtolnay
dtolnay committed Jul 7, 2024
1 parent ff022f5 commit 9809f71
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 132 deletions.
113 changes: 31 additions & 82 deletions src/classify.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,16 @@ use crate::generics::TypeParamBound;
use crate::path::{Path, PathArguments};
use crate::punctuated::Punctuated;
use crate::ty::{ReturnType, Type};
#[cfg(feature = "full")]
use proc_macro2::{Delimiter, TokenStream, TokenTree};
use std::ops::ControlFlow;

#[cfg(feature = "full")]
pub(crate) fn requires_semi_to_be_stmt(expr: &Expr) -> bool {
match expr {
Expr::Macro(expr) => !expr.mac.delimiter.is_brace(),
_ => requires_comma_to_be_match_arm(expr),
}
}

#[cfg(feature = "full")]
pub(crate) fn requires_comma_to_be_match_arm(expr: &Expr) -> bool {
match expr {
Expr::If(_)
Expand Down Expand Up @@ -60,7 +57,7 @@ pub(crate) fn requires_comma_to_be_match_arm(expr: &Expr) -> bool {
}
}

#[cfg(all(feature = "printing", feature = "full"))]
#[cfg(feature = "printing")]
pub(crate) fn confusable_with_adjacent_block(mut expr: &Expr) -> bool {
let mut stack = Vec::new();

Expand Down Expand Up @@ -146,84 +143,37 @@ pub(crate) fn confusable_with_adjacent_block(mut expr: &Expr) -> bool {
}

#[cfg(feature = "printing")]
pub(crate) fn confusable_with_adjacent_lt(mut expr: &Expr) -> bool {
pub(crate) fn trailing_unparameterized_path(mut ty: &Type) -> bool {
loop {
match expr {
Expr::Binary(e) => expr = &e.right,
Expr::Cast(e) => return trailing_unparameterized_path(&e.ty),
Expr::Reference(e) => expr = &e.expr,
Expr::Unary(e) => expr = &e.expr,

Expr::Array(_)
| Expr::Assign(_)
| Expr::Async(_)
| Expr::Await(_)
| Expr::Block(_)
| Expr::Break(_)
| Expr::Call(_)
| Expr::Closure(_)
| Expr::Const(_)
| Expr::Continue(_)
| Expr::Field(_)
| Expr::ForLoop(_)
| Expr::Group(_)
| Expr::If(_)
| Expr::Index(_)
| Expr::Infer(_)
| Expr::Let(_)
| Expr::Lit(_)
| Expr::Loop(_)
| Expr::Macro(_)
| Expr::Match(_)
| Expr::MethodCall(_)
| Expr::Paren(_)
| Expr::Path(_)
| Expr::Range(_)
| Expr::Repeat(_)
| Expr::Return(_)
| Expr::Struct(_)
| Expr::Try(_)
| Expr::TryBlock(_)
| Expr::Tuple(_)
| Expr::Unsafe(_)
| Expr::Verbatim(_)
| Expr::While(_)
| Expr::Yield(_) => return false,
}
}

fn trailing_unparameterized_path(mut ty: &Type) -> bool {
loop {
match ty {
Type::BareFn(t) => match &t.output {
ReturnType::Default => return false,
ReturnType::Type(_, ret) => ty = ret,
},
Type::ImplTrait(t) => match last_type_in_bounds(&t.bounds) {
ControlFlow::Break(trailing_path) => return trailing_path,
ControlFlow::Continue(t) => ty = t,
},
Type::Path(t) => match last_type_in_path(&t.path) {
ControlFlow::Break(trailing_path) => return trailing_path,
ControlFlow::Continue(t) => ty = t,
},
Type::Ptr(t) => ty = &t.elem,
Type::Reference(t) => ty = &t.elem,
Type::TraitObject(t) => match last_type_in_bounds(&t.bounds) {
ControlFlow::Break(trailing_path) => return trailing_path,
ControlFlow::Continue(t) => ty = t,
},
match ty {
Type::BareFn(t) => match &t.output {
ReturnType::Default => return false,
ReturnType::Type(_, ret) => ty = ret,
},
Type::ImplTrait(t) => match last_type_in_bounds(&t.bounds) {
ControlFlow::Break(trailing_path) => return trailing_path,
ControlFlow::Continue(t) => ty = t,
},
Type::Path(t) => match last_type_in_path(&t.path) {
ControlFlow::Break(trailing_path) => return trailing_path,
ControlFlow::Continue(t) => ty = t,
},
Type::Ptr(t) => ty = &t.elem,
Type::Reference(t) => ty = &t.elem,
Type::TraitObject(t) => match last_type_in_bounds(&t.bounds) {
ControlFlow::Break(trailing_path) => return trailing_path,
ControlFlow::Continue(t) => ty = t,
},

Type::Array(_)
| Type::Group(_)
| Type::Infer(_)
| Type::Macro(_)
| Type::Never(_)
| Type::Paren(_)
| Type::Slice(_)
| Type::Tuple(_)
| Type::Verbatim(_) => return false,
}
Type::Array(_)
| Type::Group(_)
| Type::Infer(_)
| Type::Macro(_)
| Type::Never(_)
| Type::Paren(_)
| Type::Slice(_)
| Type::Tuple(_)
| Type::Verbatim(_) => return false,
}
}

Expand All @@ -249,7 +199,7 @@ pub(crate) fn confusable_with_adjacent_lt(mut expr: &Expr) -> bool {
}

/// Whether the expression's first token is the label of a loop/block.
#[cfg(all(feature = "printing", feature = "full"))]
#[cfg(feature = "printing")]
pub(crate) fn expr_leading_label(mut expr: &Expr) -> bool {
loop {
match expr {
Expand Down Expand Up @@ -302,7 +252,6 @@ pub(crate) fn expr_leading_label(mut expr: &Expr) -> bool {
}

/// Whether the expression's last token is `}`.
#[cfg(feature = "full")]
pub(crate) fn expr_trailing_brace(mut expr: &Expr) -> bool {
loop {
match expr {
Expand Down
45 changes: 22 additions & 23 deletions src/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2981,6 +2981,7 @@ pub(crate) mod printing {
use crate::attr::Attribute;
#[cfg(feature = "full")]
use crate::attr::FilterAttrs;
#[cfg(feature = "full")]
use crate::classify;
#[cfg(feature = "full")]
use crate::expr::{
Expand All @@ -2996,6 +2997,7 @@ pub(crate) mod printing {
};
#[cfg(feature = "full")]
use crate::fixup::FixupContext;
#[cfg(feature = "full")]
use crate::op::BinOp;
use crate::path;
use crate::precedence::Precedence;
Expand Down Expand Up @@ -3222,17 +3224,23 @@ pub(crate) mod printing {
outer_attrs_to_tokens(&e.attrs, tokens);

#[cfg(feature = "full")]
let left_fixup = fixup.leftmost_subexpression_with_begin_operator(match &e.op {
BinOp::Sub(_)
| BinOp::Mul(_)
| BinOp::And(_)
| BinOp::Or(_)
| BinOp::BitAnd(_)
| BinOp::BitOr(_)
| BinOp::Shl(_)
| BinOp::Lt(_) => true,
_ => false,
});
let left_fixup = fixup.leftmost_subexpression_with_begin_operator(
match &e.op {
BinOp::Sub(_)
| BinOp::Mul(_)
| BinOp::And(_)
| BinOp::Or(_)
| BinOp::BitAnd(_)
| BinOp::BitOr(_)
| BinOp::Shl(_)
| BinOp::Lt(_) => true,
_ => false,
},
match &e.op {
BinOp::Shl(_) | BinOp::Lt(_) => true,
_ => false,
},
);
let left_prec = leading_precedence(
&e.left,
#[cfg(feature = "full")]
Expand All @@ -3246,21 +3254,12 @@ pub(crate) mod printing {
);

let binop_prec = Precedence::of_binop(&e.op);
let (mut left_needs_group, right_needs_group) = match binop_prec {
let (left_needs_group, right_needs_group) = match binop_prec {
Precedence::Assign => (left_prec <= Precedence::Range, right_prec < binop_prec),
Precedence::Compare => (left_prec <= binop_prec, right_prec <= binop_prec),
_ => (left_prec < binop_prec, right_prec <= binop_prec),
};

// These cases require parenthesization independently of precedence.
if let BinOp::Lt(_) | BinOp::Shl(_) = &e.op {
// `x as i32 < y` has the parser thinking that `i32 < y` is the
// beginning of a path type. It starts trying to parse `x as (i32 <
// y ...` instead of `(x as i32) < ...`. We need to convince it
// _not_ to do that.
left_needs_group |= classify::confusable_with_adjacent_lt(&e.left);
}

print_subexpression(
&e.left,
left_needs_group,
Expand Down Expand Up @@ -3341,7 +3340,7 @@ pub(crate) mod printing {
Precedence::Unambiguous
};
#[cfg(feature = "full")]
let func_fixup = fixup.leftmost_subexpression_with_begin_operator(true);
let func_fixup = fixup.leftmost_subexpression_with_begin_operator(true, false);
let func_precedence = leading_precedence(
&e.func,
#[cfg(feature = "full")]
Expand Down Expand Up @@ -3550,7 +3549,7 @@ pub(crate) mod printing {
) {
outer_attrs_to_tokens(&e.attrs, tokens);
#[cfg(feature = "full")]
let obj_fixup = fixup.leftmost_subexpression_with_begin_operator(true);
let obj_fixup = fixup.leftmost_subexpression_with_begin_operator(true, false);
let obj_precedence = leading_precedence(
&e.expr,
#[cfg(feature = "full")]
Expand Down
66 changes: 44 additions & 22 deletions src/fixup.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,15 @@ pub(crate) struct FixupContext {
// let _ = return + 1; // no paren because '+' cannot begin expr
//
next_operator_can_begin_expr: bool,

// This is the difference between:
//
// let _ = x as u8 + T;
//
// let _ = (x as u8) < T;
//
// Without parens, the latter would want to parse `u8<T...` as a type.
next_operator_can_begin_generics: bool,
}

impl FixupContext {
Expand All @@ -114,6 +123,7 @@ impl FixupContext {
parenthesize_exterior_struct_lit: false,
parenthesize_exterior_jump: false,
next_operator_can_begin_expr: false,
next_operator_can_begin_generics: false,
};

/// Create the initial fixup for printing an expression in statement
Expand Down Expand Up @@ -189,9 +199,11 @@ impl FixupContext {
pub fn leftmost_subexpression_with_begin_operator(
self,
next_operator_can_begin_expr: bool,
next_operator_can_begin_generics: bool,
) -> Self {
FixupContext {
next_operator_can_begin_expr,
next_operator_can_begin_generics,
..self.leftmost_subexpression()
}
}
Expand Down Expand Up @@ -245,38 +257,48 @@ impl FixupContext {
/// Determines the effective precedence of a left subexpression. Some
/// expressions have lower precedence when adjacent to particular operators.
pub fn leading_precedence(self, expr: &Expr) -> Precedence {
if self.next_operator_can_begin_expr {
match expr {
// Decrease precedence of value-less jumps when followed by an
// operator that would otherwise get interpreted as beginning a
// value for the jump.
Expr::Break(_) | Expr::Return(_) | Expr::Yield(_) => return Precedence::Jump,
_ => {}
match expr {
// Decrease precedence of value-less jumps when followed by an
// operator that would otherwise get interpreted as beginning a
// value for the jump.
Expr::Break(_) | Expr::Return(_) | Expr::Yield(_)
if self.next_operator_can_begin_expr =>
{
Precedence::Jump
}
Expr::Cast(cast)
if self.next_operator_can_begin_generics
&& classify::trailing_unparameterized_path(&cast.ty) =>
{
Precedence::MIN
}
_ => Precedence::of(expr),
}
Precedence::of(expr)
}

/// Determines the effective precedence of a right subexpression. Some
/// expressions have higher precedence on the right side of a binary
/// operator than on the left.
pub fn trailing_precedence(self, expr: &Expr) -> Precedence {
if !self.parenthesize_exterior_jump {
match expr {
// Increase precedence of expressions that extend to the end of
// current statement or group.
Expr::Break(_)
| Expr::Closure(_)
| Expr::Let(_)
| Expr::Return(_)
| Expr::Yield(_) => {
return Precedence::Prefix;
}
Expr::Range(e) if e.start.is_none() => return Precedence::Prefix,
_ => {}
match expr {
// Increase precedence of expressions that extend to the end of
// current statement or group.
Expr::Break(_) | Expr::Closure(_) | Expr::Let(_) | Expr::Return(_) | Expr::Yield(_)
if !self.parenthesize_exterior_jump =>
{
Precedence::Prefix
}
Expr::Range(e) if e.start.is_none() && !self.parenthesize_exterior_jump => {
Precedence::Prefix
}
Expr::Cast(cast)
if self.next_operator_can_begin_generics
&& classify::trailing_unparameterized_path(&cast.ty) =>
{
Precedence::MIN
}
_ => Precedence::of(expr),
}
Precedence::of(expr)
}
}

Expand Down
5 changes: 1 addition & 4 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,10 +329,7 @@ mod bigint;
#[cfg_attr(docsrs, doc(cfg(feature = "parsing")))]
pub mod buffer;

#[cfg(any(
all(feature = "parsing", feature = "full"),
all(feature = "printing", any(feature = "full", feature = "derive")),
))]
#[cfg(all(any(feature = "parsing", feature = "printing"), feature = "full"))]
mod classify;

mod custom_keyword;
Expand Down
2 changes: 1 addition & 1 deletion tests/test_expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ fn test_fixup() {
quote! { 0 + (0 + 0) },
quote! { (a = b) = c },
quote! { (x as i32) < 0 },
quote! { (1 + x as i32) < 0 },
quote! { 1 + (x as i32) < 0 },
quote! { (1 + 1).abs() },
quote! { (lo..hi)[..] },
quote! { (a..b)..(c..d) },
Expand Down

0 comments on commit 9809f71

Please sign in to comment.