Disallow ambiguous attributes on expressions (take 2)

[petrochenkov]

Previous reverted attempt - #124099.
cc #159581

TODO:

• Desription, crater run
• Figure out whether addressing Stabilize attributes on closure and method call expressions #159581 (comment) is necessary if macros in expr/stmt positions are not stabilized

[rustbot]

The parser was modified, potentially altering the grammar of (stable) Rust
which would be a breaking change.

cc @fmease

[rustbot]

r? @chenyukang

rustbot has assigned @chenyukang.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 75 candidates
• Random selection from 16 candidates

[petrochenkov]

Attributes in expressions, purely from syntactic point of view, at parsing time.
I.e. no distinction between active or inert, or built-in or custom attributes.
And without considering #159581 (comment).

Not syntactically suspicious (top level expressions):

Definitely can stabilize after accounting for macro attributes and #159581 (comment).

// let initializer
let x = #[attr] 10;
let x = #[attr] 10 else { ... };

// const or static item initializer
const C: u8 = #[attr] 10;
static C: u8 = #[attr] 10;

// enum discriminant value
enum E { V = #[attr] 10 }

// field default
struct S { field: u8 = #[attr] 10 }

// key value attribute
#[key = #[attr] value]

// `match` arm
match 10 {
    11 => #[attr] 12,
}

// struct literals fields
let x = Struct { field: #[attr] 10, ... }

// struct literals rest
let x = Struct { field: 10, ..#[attr] base }

// for loop iterator
for x in #[attr] 10 { ... }

// if condition
if #[attr] 10 { ... }

// while condition
while #[attr] 10 { ... }

// match scrutinee
match #[attr] 10 { ... }

// method call arguments
r.method(#[attr] 10, ...)

// array element or size
[#[attr] 10, 11, 12]
[#[attr] 10; 11]
[10; #[attr] 11]
[u8; #[attr] 11]

// function call argument
func(#[attr] 10, 11, 12)

// tuple element
(#[attr] 10, 11, 12)

// move expression
move(#[attr] 10)

// indexing argument
array[#[attr] 10]

// parentheses
(#[attr] 10)

// const generic defaults
fn foo<const C: u8 = #[attr] 10>() {}

// const parameter constraint
foo::<C = #[attr] 10>()

// const generic arguments (if supported at all)
foo::<#[attr] 10>()

// type ascription
builtin # type_ascribe(#[attr] 10, u8)

// unsafe binder casts
builtin # wrap_binder(#[attr] 10, u8)

// various inline asm operands
asm!("code", in("r") #[attr] x)

// some syntax for contracts, I didn't look closely

Semi-suspicious (top level expressions):

I don't see any actual issues, just would be interesting how often these will occur in the crater run.

Can stabilize (after accounting for macro attributes and #159581 (comment)) or not stabilize.

// closure body
let x = || #[attr] 10;

// break, return, yield, become, and yeet expressions
break #[attr] 10
return #[attr] 10
yield #[attr] 10
become #[attr] 10
do yeet #[attr] 10

// match or pattern guard conditions
pat if #[attr] true

// `expr` matcher as an await or yield receiver
$expr.await // `$expr` is `#[attr] 10`
$expr.yield // `$expr` is `#[attr] 10`

// `expr` matcher as a use receiver
$expr.use // `$expr` is `#[attr] 10`

// `expr` matcher as a field receiver
$expr.field // `$expr` is `#[attr] 10`

// `expr` matcher as a method receiver
$expr.method() // `$expr` is `#[attr] 10`

// `expr` matcher as a callable in a function call
$expr() // `$expr` is `#[attr] 10`

// `expr` matcher as an indexable expression
$expr[] // `$expr` is `#[attr] 10`

// `expr` matcher as a try expression
$expr? // `$expr` is `#[attr] 10`

Semi-suspicious (non-top level expressions):

I don't see any actual issues, just would be interesting how often these will occur in the crater run.

Can stabilize (after accounting for macro attributes and #159581 (comment)) or not stabilize.

// binary operator rhs
10 + #[attr] 11

// assignment rhs
lhs = #[attr] rhs
lhs += #[attr] rhs

// range rhs
10 .. #[attr] 11

// let expressions in if/while
if let x = #[attr] 10 && x == 11 { ... }

// prefix unary operator
- #[attr] 10

// reference operator
& #[attr] 10
& raw mut #[attr] 10

Suspicious (top level expressions):

Cannot stabilize, this is a long stanging issue with too permissive pattern parsing.

// `expr` matcher as a pattern
match 10 { $expr => {} } // `$expr` is `#[attr] 10`

// `expr` matcher in a range pattern
match 10 { $expr .. $expr => {} } // `$expr` is `#[attr] 10`

Suspicious (non-top level expressions):

Cannot stabilize.

// binary operator lhs
#[attr] 10 + 11

// assignment lhs
#[attr] lhs = rhs
#[attr] lhs += rhs

// range lhs
#[attr] 10 .. 11

// cast expressions
#[attr] 10 as u8

[petrochenkov]

Regarding #159581 (comment).

Suspicious contexts (single -> list)

In all these cases if $expr has a top level attribute inside it, it will migrate from a single to list context if the parentheses from the macro variable are not correctly preserved in AST.

// method call arguments
r.method($expr, ...)

// array element or size
[$expr, 11, 12]

// function call argument
func($expr, 11, 12)

// tuple element
($expr, 11, 12)

// const generic arguments (if supported at all)
foo::<$expr>()

Suspicious contexts (expr -> stmt)

In all these cases if $expr has a top level attribute inside it, it will migrate from a single expression to statement context if the parentheses from the macro variable are not correctly preserved in AST.

// $expr is:

// postfix await, yield or use
#[attr] val.await
#[attr] val.yield
#[attr] val.use

// field or method access
#[attr] val.field
#[attr] val.method()

// function call or indexing
#[attr] val()
#[attr] val[]

// expression
#[attr] val?

// path
#[attr] path

If an attribute starts a statement as a part of some larger expression (e.g. a binary operator), it will already be reported as an error by #160235 (comment).

Conclusion

These examples are only ambiguous if any of the involved attributes are active (including cfg), except that cfg_attr is ok (because we know what it does, and it does not depend on context).
So we don't need to gate any of this during parsing, and can continue using expansion-time gating.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[petrochenkov]

@bors try

[rust-log-analyzer]

The job aarch64-gnu-llvm-21-2 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[rust-bors]

☀️ Try build successful (CI)
Build commit: 165655a (165655a2f1981fef27b31c547ad7631979fc5139)
Base parent: 1ed2df6 (1ed2df61a19042f231709eb05d032ae9e2cb2084)

[petrochenkov]

@craterbot check

[craterbot]

👌 Experiment pr-160235 created and queued.
🤖 Automatically detected try build 165655a
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[petrochenkov]

Semi-suspicious (non-top level expressions):

I don't see any actual issues, just would be interesting how often these will occur in the crater run.

Can stabilize (after accounting for macro attributes and #159581 (comment)) or not stabilize.

There's at least one good reason to keep these unstable or unsupported (the "rhs" half including if let, not the "unary operator" half).

This gives us maximum freedom to change the attribute application priorities even inside expressions, not just at the statement start.
So things like a + #[attr] b + c become ambiguous and we can change the priorities from the current (a + #[attr] b) + c (minimum, like unary -) to a + #[attr] (b + c) (maximum, like return).

Why we may want this at all?
In other contexts attributes typically apply to the "larger thing":

// `attr` is an attribute on the whole function parameter, not on the pattern `pat`
fn foo(#[attr] pat: Type) {}

// `attr` is an attribute on the whole generic parameter, not on the type `Type`
fn foo<#[attr] Type = Default>() {}

// `attr` is an attribute on the whole where clause predicate, not on the type `Type`
fn foo() where #[attr] Type: Trait {}

// `attr` is an attribute on the whole match arm, not on the range pattern or the range start expression
match 10 {
	#[attr] 10 .. 11 => {}
}

It is reasonable to expect that the same rule works for statements (or even expressions)

// `attr` is an attribute on the whole statement, not on the lhs expression ???
#[attr]
lhs = rhs;

, but it's not actually the case, the attribute applies only to the lhs expression.
(Currently) erroneous uses like this occur in the wild.
I think in this case it would actually be reasonable to change the priorities, and say that if attributes start a statement, then they apply to the whole statement.

In purely expression contexts, like let x = #[attr] lhs + rhs or a + #[attr] b + c I wouldn't change anything and keep the current priorities ("attribute is an unary prefix operator"), but we can prohibit them anyway just to avoid confusion with other cases.

[petrochenkov]

Ah, the conclusion regarding stabilization: if the syntactic restrictions from the above are enforced, then we'll be able to stabilize inert attributes on all expressions.