Allow #[attr] if to be passed to proc macros

[Aaron1011]

The following code:

fn main() {
    #[attr] if let Some(_) = Ok(true) {}
}

produces this error:

error: attributes are not yet allowed on `if` expressions
 --> src/main.rs:2:5
  |
2 |     #[attr] if let Some(_) = Ok(true) {}
  |     ^^^^^^^

error: aborting due to previous error

Proc macros like pin-project currently use a trick to work around attributes on expressions being unstable: The entire function is annotated with a 'wrapper' attribute, which goes through and manually parses expression attributes. For example:

#[my_attr]
fn foo() {
    #[my_attr] let a = 25;
}

In the example above, the #[my_attr] let a = 25; will be processed by the outer #[my_attr] fn foo() implementation, which will strip away the expression-based #[my_attr] before emitting the final TokenStream.

Unfortunately, the current 'attributes are not yet allowed on if expressions' check is done during parsing:

rust/src/librustc_parse/parser/expr.rs

Line 679 in ac2f3fa

self.error_attr_on_if_expr(&expr);

While the proc-macro itself can parse the attribute on the if let without any errors, compilation will still fail due to the parsing error that was emitted before any proc-macros were run.

It would be extremely useful if this check were to be moved until after parsing, so that proc macros had a chance to remove attributes from if expressions.

[Centril]

From a parsing perspective the current restriction is pretty ad-hoc and annoying, as it complicates things, so if there's no good reason for it to stay, I would also like to see it go, and would be happy to review such a PR (including with tests for the parsing, pretty printing, and ideally some local relevant refactoring).

However, to make sure there are no problems, I'd like for @petrochenkov to confirm.

[ExpHP]

Yeah, to me, it seems that allowing the attribute to be present during the proc_macro call is tantamount to declaring that the syntax is legal. So the sensible change would be to make it always legal, rather than only letting it exist prior to macro expansion.

[petrochenkov]

Here's where this rule was introduced - https://github.com/rust-lang/rfcs/blob/master/text/0016-more-attributes.md#if.

[Centril]

Hmm; seems like the concern was primarily semantic (in terms of macros & lints) as opposed to syntactic?

[Aaron1011]

So the sensible change would be to make it always legal, rather than only letting it exist prior to macro expansion.

As @Centril and @petrochenkov mentioned, the semantic meaning of such an attribute is ambiguous. However, that's not a problem with accepting it syntactically, since the proc macro processing the function can interpret it however it wants.

I think it would be a good idea to allow 'normal' proc macro attrs on 'if' expressions as well (e.g. without an attribute on the entire function). The attribute invocation would be passed the entire 'if else' chain, giving the proc macro the maximum flexibility for interpreting it. The decision as to which part of the chain to actually modify would be decided by the macro implementation.

However, this would certainly require further discussion. I don't think it needs to block accepting these attributes syntactically - doing so doesn't commit us to accepting them semantically (e.g. we could still emit an error if the post-expansion code contains attributes on 'if' expressions.