Insert parentheses around binary operation with attribute

[dtolnay]

Fixes the bug found by @fmease in #134661 (review).

Previously, -Zunpretty=expanded would expand this program as follows:

#![feature(stmt_expr_attributes)]
#![allow(unused_attributes)]

macro_rules! group {
    ($e:expr) => {
        $e
    };
}

macro_rules! extra {
    ($e:expr) => {
        #[allow()] $e
    };
}

fn main() {
    let _ = #[allow()] 1 + 1;
    let _ = group!(#[allow()] 1) + 1;
    let _ = 1 + group!(#[allow()] 1);
    let _ = extra!({ 0 }) + 1;
    let _ = extra!({ 0 } + 1);
}

let _ = #[allow()] 1 + 1;
let _ = #[allow()] 1 + 1;
let _ = 1 + #[allow()] 1;
let _ = #[allow()] { 0 } + 1;
let _ = #[allow()] { 0 } + 1;

The first 4 statements are the correct expansion, but the last one is not. The attribute is supposed to apply to the entire binary operation, not only to the left operand.

After this PR, the 5th statement will expand to:

let _ = #[allow()] ({ 0 } + 1);

In the future, as some subset of stmt_expr_attributes approaches stabilization, it is possible that we will need to do parenthesization for a number of additional cases depending on the outcome of #127436. But for now, at least this PR makes the pretty-printer align with the current behavior of the parser.

r? fmease

[rustbot]

fmease is not on the review rotation at the moment.
They may take a while to respond.

[dtolnay]

r? compiler

[bors]

☔ The latest upstream changes (presumably #142550) made this pull request unmergeable. Please resolve the merge conflicts.

[fmease]

The explanation you've given in https://github.com/rust-lang/rust/pull/134661/files#r2143009318 makes perfect sense, thanks!

[fmease]

It makes me slightly uneasy that we have to "hard code" a collection of expr kinds separately here but rn I don't see a more robust alternative that's only based on (effective) precedence levels.

[fmease]

This assortment of expr kinds is lacking ast::Expr::Range. Consider:

macro_rules! attach { ($e:expr) => { #[allow()] $e }
fn main() { _ = attach!(0..1); }

Current output is #[allow()] 0..1 which actually means (#[allow()] 0)..1. So it should be printed like so instead: #[allow()] (0..1).

As for ranges with no lower bound ..expr, ..=expr and .., the same thing applies since apparently, #[attr] ..expr (etc.) is syntactically invalid and requires parens #[attr] (..expr) (etc.).

[fmease]

Thanks a lot! r=me with Ranges included (with additional tests ofc), squash or don't idc @bors rollup

[fmease]

That makes total sense and it's still insane!

[dtolnay]

Fun fact, syn's ToTokens impls as well as prettyplease both implement this edge case, and will print with the minimal inserted parentheses, producing return x + .. .field in this case.

[dtolnay]

@bors r=fmease

[bors]

📌 Commit 535e11f has been approved by fmease

It is now in the queue for this repository.