Stabilize attributes on closure and method call expressions

[Unique-Usman]

View all comments

Stabilization report

Summary

This PR partially stabilizes outer attributes on expressions under the existing
stmt_expr_attributes feature.

It stabilizes attributes directly on closure expressions and method-call
expressions, including parenthesized forms and complete method-call chains:

let closure = #[inline] || 42;let len = #[allow(unused)] value.len();let result = #[allow(unused)] value.first().second();let result = #[allow(unused)](value.first().second());

RFC 16 proposed broader support for attributes on statements and expressions.
Several unambiguous positions have already become stable, including statements,
blocks, match arms, and elements of arrays, tuples, and function argument lists.
General expression attributes remain unstable because it may be unclear which
expression an initial attribute applies to.

This stabilization selects closures and method calls, whose attribute targets
are clear, while leaving the remaining expression and subexpression cases
behind stmt_expr_attributes.

Tracking and related work:

• RFC 16: More attributes
• stmt_expr_attributes tracking issue #15701
• Summary of the remaining design questions
• Path-forward discussion #127436

Two I opened together with this one for reference update and the for rejecting cfg one expressions that cannot be safely remove:

• Reference PR #2314
• Reject cfg on expressions that cannot be safely removed #159580

cc @rust-lang/lang @rust-lang/lang-advisors @rust-lang/lang-docs @rust-lang/compiler

What is stabilized

Closure expressions

Outer attributes may be placed directly on closures without enabling
stmt_expr_attributes:

let closure = #[inline] || 42;let cold_closure = #[cold]move || perform_work();

This stabilizes the attribute position. Existing target validation still
determines whether a particular attribute may be applied to a closure.

Method-call expressions

Outer attributes may be placed directly on method-call expressions:

let len = #[allow(unused)] value.len();

For a method-call chain, the attribute applies to the complete resulting
method-call expression:

let result = #[allow(unused)] value.first().second();

Parentheses around a closure or method call do not change whether the position
is stable:

let result = #[allow(unused)](value.first().second());

What is not stabilized

The stmt_expr_attributes feature itself remains unstable. Attributes on other
expression kinds continue to require the feature gate.

For example, these remain unstable:

let value = #[allow(unused)]42;let value = #[allow(unused)]free_function();let value = #[allow(unused)] object.method().field;let value = #[allow(unused)] values[0];let value = #[allow(unused)]1 + 2;

Although object.method().field contains a method call, its outer expression is
a field expression, so it is outside this stabilization.

This PR does not decide the behavior of attributes on arbitrary literals,
function calls, field expressions, indexing, binary expressions, or other
remaining expression and subexpression positions.

There is also a detailed summary report I created after deeply studying what was stabilized, what has not been stabilized and where were they confirmed to be stabilized and the summary can be find here: - #15701 (comment)

Design

The primary concern with general expression attributes is identifying the
expression to which an initial attribute belongs. For example:

#[attr] a + b

can raise readability and precedence questions about whether the attribute
belongs to a or to the complete binary expression.

Closures have a distinct syntactic introducer:

#[attr] || expression

Method calls form a clear postfix expression:

#[attr] receiver.method()

For chained calls, the complete chain is represented by an outer method-call
expression. The attribute consequently applies to the complete chain.

This partial stabilization does not introduce additional feature gates. The
existing stmt_expr_attributes gate remains active for all expression kinds
outside the stated boundary.

Interaction with cfg

Stabilizing an attribute position does not mean that #[cfg] may remove an
expression required by its parent.

For example:

let value = #[cfg(false)] object.method();

cannot be configured away because that would leave the initializer incomplete.
It continues to produce:

removing an expression is not supported in this position

rust-lang/rust#159580
ensures that cfg on expressions that cannot be safely removed is rejected
regardless of whether the configuration evaluates to true or false.

Positions where cfg can safely remove a complete element, such as an array,
tuple, or function argument element, are unaffected.

Implementation

The parser and AST already support this syntax, so this stabilization does not
change the grammar or AST representation.

The expression-attribute feature-gate checker now receives the attributed
ast::Expr. Before emitting the stmt_expr_attributes diagnostic, it
recognizes:

• ExprKind::Closure
• ExprKind::MethodCall
• ExprKind::Paren containing either of those expressions

The expression node is passed through the relevant ordinary expansion,
optional-expression expansion, and cfg_eval configuration paths so that they
all apply the same stability rule.

The stmt_expr_attributes entry remains in
compiler/rustc_feature/src/unstable.rs; it is not moved to accepted.rs.

Test coverage

Positive tests cover:

• Direct attributes on closures.
• A single method call.
• Complete method-call chains.
• Parenthesized method-call chains.
• Closure and method-call attributes processed through cfg_eval.
• Existing #[inline], #[cold], #[coroutine], capture-analysis, and
  #[track_caller] uses on closures.

Negative tests verify that the following remain gated:

• Literals.
• Free-function calls.
• Field expressions following method calls.
• Index expressions.
• Binary expressions.

Configuration tests verify that a required method-call expression cannot be
removed with #[cfg(false)].

Existing closure-analysis tests were updated only to remove their obsolete E0658
expectations. Their intended capture-analysis diagnostics remain. Coroutine
tests no longer enable stmt_expr_attributes solely to place #[coroutine] on
a closure.

Compatibility and language interactions

This is an additive change. Programs previously accepted on stable Rust remain
accepted, so a crater run is not expected to be necessary.

This stabilization:

• Does not change type inference or method resolution.
• Does not introduce new expressions, temporaries, or evaluation steps.
• Does not change temporary scopes or drop order.
• Does not affect operational semantics or unsafe-code rules.
• Is not edition-dependent.
• Does not stabilize cfg_eval, coroutines, or individual unstable attributes.
• Preserves existing attribute target validation.
• Preserves stmt_expr_attributes for all other expression kinds.

Tools and documentation

No Cargo, rustdoc, Clippy, rustup, or docs.rs changes are required because no new
syntax or AST form is introduced. The syntax was already available on nightly.

Rustfmt and rust-analyzer use the existing expression-attribute syntax, but
their behavior should be confirmed during review.

Reference PR #2314:

• Lists closures and method calls as valid expression-attribute positions.
• Specifies that an attribute on a method-call chain applies to the complete
  method-call expression.
• Updates the inline, cold, and instruction_set documentation so that it
  no longer describes direct closure expression attributes as unsupported.

The Reference PR must be reviewed and approved by the lang-docs team before this
stabilization is merged.

Outstanding concerns

No known implementation or soundness issue specifically blocks this narrow
stabilization.

The broader design questions tracked in
#15701 and
#127436 remain relevant to
the expression kinds that are not being stabilized.

An existing FIXME notes differences between ordinary and optional expression
feature-gating paths. Both paths are updated and tested by this implementation,
so it does not block this stabilization.

Contributors and prior work

The current work builds on contributions from:

• @huonw — author of RFC 16;
• @brson — opened the expression-attribute tracking issue and merged the
  original RFC;
• @Kimundi — author of the initial implementation in
  [rust-lang/rust#29850](Implements RFC 16, attributes on statements and expressions. #29850);
• @nrc — author of the statement-attribute stabilization in
  [rust-lang/rust#36995](stabilise ?, attributes on stmts, deprecate Reflect #36995);
• @WaffleLapkin and @traviscross — the proposal and discussion for splitting
  the remaining feature into narrower stabilization candidates;
• @dtolnay and @fmease — expression attribute precedence work;
• @jonas-schievink — earlier Reference documentation for closure attributes;
• @Unique-Usman — the current stabilization implementation, #[cfg] safety
  fix, tests, and Reference update.

Acknowledgments

Thanks to the authors and implementers of RFC 16 and to everyone involved in
the expression-attribute discussions, implementations, and previous
stabilizations, including the contributors to #15701, #32796, #36995, #127436,
#134661, #159580, and the corresponding Reference work. Thanks to my rust compiler mentor @estebank and thanks to @WaffleLapkin for helping.

[rustbot]

Some changes occurred in tests/ui/sanitizer

cc @rcvalle

[rustbot]

r? @folkertdev

rustbot has assigned @folkertdev.
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 74 candidates
• Random selection from 16 candidates

[folkertdev]

We generally do reference updates separately. you can open a PR over there and link it to this one.

View changes since the review

[Unique-Usman]

Noted, thanks.

[rust-log-analyzer]

The job pr-check-2 failed! Check out the build log: (web)(plain enhanced)(plain)

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

[RalfJung]

Some of this already is stable, isn't it? This works on stable:

fnfoo(x:Option<i32>){
x.map(#[inline] |y| y+1);// does not work: let closure = (#[inline] || 42);}

OTOH with a let-bound closure this does not work, not even if I add parentheses around it.

So, looks like attributes on expressions are already partially stabilized, but this stabilizes them a bit more?

[mejrs]

OTOH with a let-bound closure this does not work, not even if I add parentheses around it.

You have to add curlies, it's also allowed for the trailing expression in a block:

let _x = {#[inline]
|| ()};

but tool attributes are not allowed there on stable

let _x = {#[rustfmt::skip]
|| ()};

[Unique-Usman]

Some of this already is stable, isn't it? This works on stable:

fnfoo(x:Option<i32>){
x.map(#[inline] |y| y+1);// does not work: let closure = (#[inline] || 42);}

This form is already stabilized, this is a summary of what has already being stabilized before this pr #15701 (comment)

OTOH with a let-bound closure this does not work, not even if I add parentheses around it.

So, looks like attributes on expressions are already partially stabilized, but this stabilizes them a bit more?

Exactly, it stabilized unambiguous expression forms.

[petrochenkov]

This will need to be audited by me for historical context and macros, and by @fmease for the current parser state.

Using something being a closure or a method call as a criterion seems to be a distraction, if something like

#[attr]
method.call() + rhs;

starts working after this PR, it means the core ambiguity preventing the stabilization is not resolved.
Unless it was resolved earlier by something like #124099, I'm not sure what the current state is, need to look in more detail.
(And if it was indeed resolved, then we can stabilize this for any "tightly grouped" expressions, not just method calls and closures.)

UPD: #124099 was reverted due to breakage on real code, things are going to be very sad.

[Unique-Usman]

This will need to be audited by me for historical context and macros, and by @fmease for the current parser state.

Using something being a closure or a method call as a criterion seems to be a distraction, if something like

#[attr]
method.call() + rhs;

This is already being stabilized before this pr.

starts working after this PR, it means the core ambiguity preventing the stabilization is not resolved. Unless it was resolved earlier by something like #124099, I'm not sure what the current state is, need to look in more detail. (And if it was indeed resolved, then we can stabilize this for any "tightly grouped" expressions, not just method calls and closures.)

Thanks for the comment, definitely, auditing the pr will really be appreciated.

I have actually done a sort of deep study and understanding of what has already being stabilzed and what was not stabilized and for what reason they were not stabilized and also added the relevant links or discussions which confirms what was stabilized or what was not and I wrote a detailed summary for that here: #15701 (comment) and also suggested a pathway forward and which lead to me working on this pr and the fact that some people show interest in it.

I will add the link of the summary to the stabilization report.

Thanks once again.

[mejrs]

#[attr]
method.call() + rhs;

This is already being stabilized before this pr.

Where? I don't think this should be stabilized,

[Unique-Usman]

#[attr]
method.call() + rhs;

This is already being stabilized before this pr.

Where? I don't think this should be stabilized,

oops, confirming it again, I was using the rust compiled version of this pr. @petrochenkov, thanks for pointing this out. This pr should not stabilized attribute of this particular expression.

[WaffleLapkin]

Please split-out the change to what the feature allows from the stabilization itself. Having them in the same PR complicates things unnecessarily.

View changes since this review

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[Unique-Usman]

Please split-out the change to what the feature allows from the stabilization itself. Having them in the same PR complicates things unnecessarily.

View changes since this review

Do you mean, having a seperate feature gate for the features stabilized by this pr ?

[WaffleLapkin]

@Unique-Usman I mean changing the existing gate to only allow things that we want to stabilize.

[Unique-Usman]

@Unique-Usman I mean changing the existing gate to only allow things that we want to stabilize.

@WaffleLapkin sorry, again, please can kindly break it down, I am not getting it. Thanks.

[WaffleLapkin]

@Unique-Usman I want you to change what stmt_expr_attributes feature allows, such that it allows only the things that we want to stabilize. I.e. only allow the subset that you wanted to stabilize in this PR.

Then you will have two PRs:

1. Changing what stmt_expr_attributes allows
2. Stabilizing stmt_expr_attributes fully

[petrochenkov]

I'm not yet convinced this is the subset that we can or want to stabilize, so it's probably premature to make the changes.
(I started auditing the parser code yesterday, and found that #124099 was reverted in particular, which is not good. I'll need ~one more day to finish, probably tomorrow or on Tuesday).

[Unique-Usman]

@Unique-Usman I want you to change what stmt_expr_attributes feature allows, such that it allows only the things that we want to stabilize. I.e. only allow the subset that you wanted to stabilize in this PR.

Then you will have two PRs:

1. Changing what stmt_expr_attributes allows
2. Stabilizing stmt_expr_attributes fully

Noted and thanks.

[Unique-Usman]

I'm not yet convinced this is the subset that we can or want to stabilize, so it's probably premature to make the changes. (I started auditing the parser code yesterday, and found that #124099 was reverted in particular, which is not good. I'll need ~one more day to finish, probably tomorrow or on Tuesday).

yep, I will wait for your auditing. Thanks for the help.

[rust-bors]

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

[petrochenkov]

This topic is also closely related to this issue

• Tracking issue for migrating all remaining post-expansion feature gates to pre-expansion ones #154045

stmt_expr_attributes is a syntax feature, but it's currently gated either post-expansion for inert attributes, or during expansion for non built-in attributes and cfg, or not gated at all. That's where most of the existing stability holes come from.
We need to implement the pre-expansion gating, together with ambiguity detection, and run it through crater.

[petrochenkov]

Another ambiguous case not addressed by #124099 and related to this issue

• rustc_parser incorrectly parses groups with Delimiter::None #67062

#[attr1]
$expr;

where $expr is something like #[attr2] 10.

In the "attribute is always an unary prefix operator" model the example above will contain two expression attributes, in the "attributes starting a statement are always statement attributes" the example will instead contain one statement attribute and one expression attribute inside the invisible parentheses introduced by $expr.
I suspect that #67062 is actually easily fixable for expressions, types and patterns (#67062 (comment)), but until it's done we can just avoid stabilizing cases like this.

[Unique-Usman]

@petrochenkov thanks for the follow up, the question now is what is your suggestion to get this pr to land ? thanks

[petrochenkov]

This is waiting on me now, I planned to implement the suggestion from #159581 (comment) today and start crater, but didn't get to it.

[Unique-Usman]

This is waiting on me now, I planned to implement the suggestion from #159581 (comment) today and start crater, but didn't get to it.

ok, I will be expecting.

[petrochenkov]

This is waiting on me now, I planned to implement the suggestion from #159581 (comment) today and start crater, but didn't get to it.

#160235
@rustbot blocked