handle paren in macro expand for let-init-else expr

[bvanjoi]

Fixes #131655

This PR modifies the codegen logic of the macro expansion within let-init-else expression:

• Before: The expression let xxx = (mac! {}) else {} expands to let xxx = (expanded_ast) else {}.
• After: The same expression expands to let xxx = expanded_ast else {}.

An alternative solution to this issue could involve handling the source code directly when encountering unused parentheses in let-init-else expressions. However, this approach might be more cumbersome due to the absence of the necessary data structure.

r? @petrochenkov

[cjgillot]

This seems like a far reaching change because of a buggy lint.
Would a targeted exception on the lint itself be simpler?

[bvanjoi]

Would a targeted exception on the lint itself be simpler?

If we follow this thought, we need to check the original AST for init_expr in let-init-else when encountering unused_paren. There are two possible approaches:

• Parse the expression again to check if it is a macro call with braces.
• Record the node_id if the paren expression contains a macro call with braces during expansion.

[petrochenkov]

This is an issue with any context that restricts braces, right?
Like plain if:

fn main() {
    macro_rules! x {
        () => { true }
    }

    if (x! {}) {} // warning: unnecessary parentheses around `if` condition
}

See NO_STRUCT_LITERAL in the parser for all such contexts.
IIRC, braced macro calls like mac! {} are currently prohibited in all such contexts for consistency with other braced syntaxes (even if it's not technically necessary) due to a relatively recent lang team decision.

[dtolnay]

if (x! {}) {} is actually unnecessary parentheses. if x! {} {} is valid syntax.

[petrochenkov]

Perhaps the lint should look at span of the expression inside the parentheses, check what macro expansion it comes from, and suppress the warning in suspicious cases.
The current implementation in this PR is certainly not the way to go.

IIRC @dtolnay was also recently working on pretty-printing / linting (un)necessary parentheses, maybe he has more suggestions.

[bvanjoi]

Update: It now stores the span of necessary parenthesis nodes and uses it when checking for unused_parens lint.

[bvanjoi]

@rustbot ready

[bors]

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

[petrochenkov]

Sorry for the long delay.

I think the current version is still too much.
This can probably be fixed by a simpler heuristic local to compiler/rustc_lint/src/unused.rs - in if expr { ... } we look 1) at the span of expr and 2) at the span of the whole if expr { ... }, and if syntactic contexts of the spans do not match then we suppress the lint.
Something like Span::contains or similar could also be used instead of context comparison.
Other lints and diagnostics are full of checks like this.
@rustbot author