diff options
| author | Jacob Pratt <jacob@jhpratt.dev> | 2024-12-21 01:18:43 -0500 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-12-21 01:18:43 -0500 |
| commit | ea8bc3b4bee0e4ea2cdef8e24de0a531f2e785a7 (patch) | |
| tree | f2cc566add2d2ce511e5b2e9103ef72fda8159e4 /tests | |
| parent | cc27e3f08bf05d80fbfd4d165521ab5eb26fd817 (diff) | |
| parent | fe65e886f30ab4e045e8e492e08f7c5e3a73129a (diff) | |
| download | rust-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 'tests')
| -rw-r--r-- | tests/ui-fulldeps/pprust-parenthesis-insertion.rs | 14 |
1 files changed, 10 insertions, 4 deletions
diff --git a/tests/ui-fulldeps/pprust-parenthesis-insertion.rs b/tests/ui-fulldeps/pprust-parenthesis-insertion.rs index 184458bad55..258a1fdb1d3 100644 --- a/tests/ui-fulldeps/pprust-parenthesis-insertion.rs +++ b/tests/ui-fulldeps/pprust-parenthesis-insertion.rs @@ -61,6 +61,9 @@ static EXPRS: &[&str] = &[ "(2 + 2) * 2", "2 * (2 + 2)", "2 + 2 + 2", + // Right-associative operator. + "2 += 2 += 2", + "(2 += 2) += 2", // Return has lower precedence than a binary operator. "(return 2) + 2", "2 + (return 2)", // FIXME: no parenthesis needed. @@ -89,6 +92,13 @@ static EXPRS: &[&str] = &[ // allowed, except if the break is also labeled. "break 'outer 'inner: loop {} + 2", "break ('inner: loop {} + 2)", + // Grammar restriction: ranges cannot be the endpoint of another range. + "(2..2)..2", + "2..(2..2)", + "(2..2)..", + "..(2..2)", + // Grammar restriction: comparison operators cannot be chained (1 < 2 == false). + "((1 < 2) == false) as usize", // Grammar restriction: the value in let-else is not allowed to end in a // curly brace. "{ let _ = 1 + 1 else {}; }", @@ -113,10 +123,6 @@ static EXPRS: &[&str] = &[ "if let _ = () && (Struct {}).x {}", */ /* - // FIXME: pretty-printer produces invalid syntax. `(1 < 2 == false) as usize` - "((1 < 2) == false) as usize", - */ - /* // FIXME: pretty-printer produces invalid syntax. `for _ in 1..{ 2 } {}` "for _ in (1..{ 2 }) {}", */ |
