This repository was archived by the owner on May 28, 2025. It is now read-only.
Commit 89e2160
committed
Auto merge of rust-lang#119105 - dtolnay:paren, r=WaffleLapkin
Fix parenthesization of subexprs containing statement boundary
This PR fixes a multitude of false negatives and false positives in the AST pretty printer's parenthesis insertion related to statement boundaries — statements which terminate unexpectedly early if there aren't parentheses.
Without this fix, the AST pretty printer (including both `stringify!` and `rustc -Zunpretty=expanded`) is prone to producing output which is not syntactically valid Rust. Invalid output is problematic because it means Rustfmt is unable to parse the output of `cargo expand`, for example, causing friction by forcing someone trying to debug a macro into reading poorly formatted code.
I believe the set of bugs fixed in this PR account for the most prevalent reason that `cargo expand` produces invalid output in real-world usage.
Fixes rust-lang#98790.
## False negatives
The following is a correct program — `cargo check` succeeds.
```rust
macro_rules! m {
($e:expr) => {
match () { _ => $e }
};
}
fn main() {
m!({ 1 } - 1);
}
```
But `rustc -Zunpretty=expanded main.rs` produces output that is invalid Rust syntax, because parenthesization is needed and not being done by the pretty printer.
```rust
fn main() { match () { _ => { 1 } - 1, }; }
```
Piping this expanded code to rustfmt, it fails to parse.
```console
error: unexpected `,` in pattern
--> <stdin>:1:38
|
1 | fn main() { match () { _ => { 1 } - 1, }; }
| ^
|
help: try adding parentheses to match on a tuple...
|
1 | fn main() { match () { _ => { 1 } (- 1,) }; }
| + +
help: ...or a vertical bar to match on multiple alternatives
|
1 | fn main() { match () { _ => { 1 } - 1 | }; }
| ~~~~~
```
Fixed output after this PR:
```rust
fn main() { match () { _ => ({ 1 }) - 1, }; }
```
## False positives
Less problematic, but worth fixing (just like rust-lang#118726).
```rust
fn main() {
let _ = match () { _ => 1 } - 1;
}
```
Output of `rustc -Zunpretty=expanded lib.rs` before this PR. There is no reason parentheses would need to be inserted there.
```rust
fn main() { let _ = (match () { _ => 1, }) - 1; }
```
After this PR:
```rust
fn main() { let _ = match () { _ => 1, } - 1; }
```
## Alternatives considered
In this PR I opted to parenthesize only the leading subexpression causing the statement boundary, rather than the entire statement. Example:
```rust
macro_rules! m {
($e:expr) => {
$e
};
}
fn main() {
m!(loop { break [1]; }[0] - 1);
}
```
This PR produces the following pretty-printed contents for fn main:
```rust
(loop { break [1]; })[0] - 1;
```
A different equally correct output would be:
```rust
(loop { break [1]; }[0] - 1);
```
I chose the one I did because it is the *only* approach used by handwritten code in the standard library and compiler. There are 4 places where parenthesization is being used to prevent a statement boundary, and in all 4, the developer has chosen to parenthesize the smallest subexpression rather than the whole statement:
https://github.com/rust-lang/rust/blob/b37d43efd9c5a7a3d76ed21c454dd0f40945d77d/compiler/rustc_codegen_cranelift/example/alloc_system.rs#L102
https://github.com/rust-lang/rust/blob/b37d43efd9c5a7a3d76ed21c454dd0f40945d77d/compiler/rustc_parse/src/errors.rs#L1021-L1029
https://github.com/rust-lang/rust/blob/b37d43efd9c5a7a3d76ed21c454dd0f40945d77d/library/core/src/future/poll_fn.rs#L151
https://github.com/rust-lang/rust/blob/b37d43efd9c5a7a3d76ed21c454dd0f40945d77d/library/core/src/ops/range.rs#L824-L828File tree
3 files changed
+358
-44
lines changed- compiler/rustc_ast_pretty/src/pprust
- state
- tests/ui/macros
3 files changed
+358
-44
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1096 | 1096 | | |
1097 | 1097 | | |
1098 | 1098 | | |
1099 | | - | |
| 1099 | + | |
| 1100 | + | |
| 1101 | + | |
| 1102 | + | |
| 1103 | + | |
1100 | 1104 | | |
1101 | 1105 | | |
1102 | 1106 | | |
1103 | 1107 | | |
1104 | 1108 | | |
1105 | 1109 | | |
1106 | | - | |
| 1110 | + | |
| 1111 | + | |
| 1112 | + | |
| 1113 | + | |
| 1114 | + | |
1107 | 1115 | | |
1108 | 1116 | | |
1109 | 1117 | | |
| |||
1155 | 1163 | | |
1156 | 1164 | | |
1157 | 1165 | | |
1158 | | - | |
| 1166 | + | |
| 1167 | + | |
| 1168 | + | |
| 1169 | + | |
| 1170 | + | |
1159 | 1171 | | |
1160 | 1172 | | |
1161 | 1173 | | |
| |||
0 commit comments