about summary refs log tree commit diff
path: root/compiler/rustc_parse/src/parser
diff options
context:
space:
mode:
authorJacob Pratt <jacob@jhpratt.dev>2024-12-21 01:18:43 -0500
committerGitHub <noreply@github.com>2024-12-21 01:18:43 -0500
commitea8bc3b4bee0e4ea2cdef8e24de0a531f2e785a7 (patch)
treef2cc566add2d2ce511e5b2e9103ef72fda8159e4 /compiler/rustc_parse/src/parser
parentcc27e3f08bf05d80fbfd4d165521ab5eb26fd817 (diff)
parentfe65e886f30ab4e045e8e492e08f7c5e3a73129a (diff)
downloadrust-ea8bc3b4bee0e4ea2cdef8e24de0a531f2e785a7.tar.gz
rust-ea8bc3b4bee0e4ea2cdef8e24de0a531f2e785a7.zip
Rollup merge of #134600 - dtolnay:chainedcomparison, r=oli-obk
Fix parenthesization of chained comparisons by pretty-printer

Example:

```rust
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; }`

```console
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:

- https://github.com/rust-lang/rust/blob/13170cd787cb733ed24842ee825bcbd98dc01476/compiler/rustc_ast_pretty/src/pprust/state/expr.rs#L283-L287

- https://github.com/rust-lang/rust/blob/13170cd787cb733ed24842ee825bcbd98dc01476/compiler/rustc_hir_pretty/src/lib.rs#L1295-L1299

- https://github.com/rust-lang/rust/blob/13170cd787cb733ed24842ee825bcbd98dc01476/compiler/rustc_parse/src/parser/expr.rs#L282-L289

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.
Diffstat (limited to 'compiler/rustc_parse/src/parser')
-rw-r--r--compiler/rustc_parse/src/parser/expr.rs12
1 files changed, 2 insertions, 10 deletions
diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs
index 2f4adf2af9e..7533e75ffe2 100644
--- a/compiler/rustc_parse/src/parser/expr.rs
+++ b/compiler/rustc_parse/src/parser/expr.rs
@@ -279,13 +279,9 @@ impl<'a> Parser<'a> {
                 break;
             }
 
-            let fixity = op.fixity();
-            let min_prec = match fixity {
+            let min_prec = match op.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),
+                Fixity::Left | Fixity::None => Bound::Excluded(prec),
             };
             let (rhs, _) = self.with_res(restrictions - Restrictions::STMT_EXPR, |this| {
                 let attrs = this.parse_outer_attributes()?;
@@ -337,10 +333,6 @@ impl<'a> Parser<'a> {
                     self.dcx().span_bug(span, "AssocOp should have been handled by special case")
                 }
             };
-
-            if let Fixity::None = fixity {
-                break;
-            }
         }
 
         Ok((lhs, parsed_something))