Fix parenthesization of chained comparisons by pretty-printer

[dtolnay]

Example:

macro_rules! repro {
    () => {
        1 < 2
    };
}

fn main() {
    let _ = repro!() == false;
}

Previously -Zunpretty=expanded would pretty-print this syntactically invalid output: fn main() { let _ = 1 < 2 == false; }

error: comparison operators cannot be chained
 --> <anon>:8:23
  |
8 | fn main() { let _ = 1 < 2 == false; }
  |                       ^   ^^
  |
help: parenthesize the comparison
  |
8 | fn main() { let _ = (1 < 2) == false; }
  |                     +     +

With the fix, it will print fn main() { let _ = (1 < 2) == false; }.

Making -Zunpretty=expanded consistently produce syntactically valid Rust output is important because that is what makes it possible for cargo expand to format and perform filtering on the expanded code.

Review notes

According to rg '\.fixity\(\)' compiler/ the fixity function is called only 3 places:

• rust/compiler/rustc_ast_pretty/src/pprust/state/expr.rs

  Lines 283 to 287 in 13170cd

  	let (mut left_needs_paren, right_needs_paren) = match assoc_op.fixity() {
  	Fixity::Left => (left_prec < binop_prec, right_prec <= binop_prec),
  	Fixity::Right => (left_prec <= binop_prec, right_prec < binop_prec),
  	Fixity::None => (left_prec <= binop_prec, right_prec <= binop_prec),
  	};

• rust/compiler/rustc_hir_pretty/src/lib.rs

  Lines 1295 to 1299 in 13170cd

  	let (mut left_needs_paren, right_needs_paren) = match assoc_op.fixity() {
  	Fixity::Left => (left_prec < binop_prec, right_prec <= binop_prec),
  	Fixity::Right => (left_prec <= binop_prec, right_prec < binop_prec),
  	Fixity::None => (left_prec <= binop_prec, right_prec <= binop_prec),
  	};

• rust/compiler/rustc_parse/src/parser/expr.rs

  Lines 282 to 289 in 13170cd

  	let fixity = op.fixity();
  	let min_prec = match fixity {
  	Fixity::Right => Bound::Included(prec),
  	Fixity::Left => Bound::Excluded(prec),
  	// We currently have no non-associative operators that are not handled above by
  	// the special cases. The code is here only for future convenience.
  	Fixity::None => Bound::Excluded(prec),
  	};

The 2 pretty printers definitely want to treat comparisons using Fixity::None. That's the whole bug being fixed. Meanwhile, the parser's Fixity::None codepath is previously unreachable as indicated by the comment, so as long as Fixity::None here behaves exactly the way that Fixity::Left used to behave, you can tell that this PR definitely does not constitute any behavior change for the parser.

My guess for why comparison operators were set to Fixity::Left instead of Fixity::None is that it's a very old workaround for giving a good chained comparisons diagnostic (like what I pasted above). Nowadays that is handled by a different dedicated codepath.

[rustbot]

r? @oli-obk

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[oli-obk]

@bors r+ rollup

[bors]

📌 Commit fe65e88 has been approved by oli-obk

It is now in the queue for this repository.