rust/compiler/rustc_ast_pretty
bors 89e2160c4c Auto merge of #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 #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 #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:

b37d43efd9/compiler/rustc_codegen_cranelift/example/alloc_system.rs (L102)

b37d43efd9/compiler/rustc_parse/src/errors.rs (L1021-L1029)

b37d43efd9/library/core/src/future/poll_fn.rs (L151)

b37d43efd9/library/core/src/ops/range.rs (L824-L828)
2023-12-27 21:27:26 +00:00
..
src Auto merge of #119105 - dtolnay:paren, r=WaffleLapkin 2023-12-27 21:27:26 +00:00
Cargo.toml Remove IterDelimited. 2023-11-22 08:13:21 +11:00