optimize attribute applied to things other than methods/functions/c…

[Borgerr]

…losures gives an error (#128488)

Relevant issues:

• optimize attribute can be applied to anything #128488
• Tracking issue for RFC 2412, "The optimize attribute" #54882

PR changes attribute to only apply to functions, methods, and closures by failing compilation rather than giving a warning. This relates back to this relevant comment.

This PR should absolutely be merged only after a consensus by those with say over the RFC. The purpose of the PR is to allow a simple button press for accomplishing the task.

Cc. @Centril @clubby789 @oli-obk
r? rust-lang/diagnostics

[jieyouxu]

r? @jieyouxu

This needs a T-lang FCP since it diverges from the original RFC 2412 specified behavior. cc @rust-lang/lang

Summary for T-lang

This PR proposes to elevate #[optimize] attribute (as specified in RFC 2412) applied to unsupported HIR nodes from a warn-by-default unused_attributes lint to a hard error.

The RFC explains the rationale for emitting a warn-by-default unused_attributes lint as (emphasis mine with minor corrections)

#[optimize] attribute applied to non function-like items (such as struct) or non function-like expressions (i.e. not closures) is considered “unused” as of this RFC and should fire the unused_attributes lint (unless the same attribute was used for a function-like item or expression, via e.g. propagation). Some future RFC may assign some behaviour to this attribute with respect to such definitions.

Presumably, this was only designated as a warn-by-default unused_attribute lint to allow future extensions for the #[optimize] attribute for more HIR nodes?

In particular, the RFC and the tracking issue #54882 states an unresolved question (edited by me for correct lint name):

Are the propagation and unused_attributes approaches right?

Other attributes such as #[naked] reports a hard error if it is misapplied to HIR nodes that it does not intend to target.

Relevant context

• optimize attribute can be applied to anything #128488 noticed that #[optimize] could be applied to arbitrary HIR nodes.
• Emit an error if #[optimize] is applied to an incompatible item #128458 added a warn-by-default unused_attributes lint as specified by the RFC.

@rustbot label +I-lang-nominated

[Borgerr]

Forgot to remove the FIXME comment...

[bors]

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

[compiler-errors]

Hi @Borgerr, I'd like to run a crater run to determine the fallout of this becoming a hard error. Could you rebase this PR, then we can kick that off?

[nikomatsakis]

In general I prefer to give a hard error so that we can change the meaning rather than silently change behavior in the future, though sometimes practical considerations (i.e., widespread usage) make that not worth it. I doubt this attribute is heavily used however so I'm in favor of this change, assuming crater doesn't get too grouchy as a result.

[compiler-errors]

Actually, no this doesn't need a crater run. Can you rebase this, though, so it can be approved? :D

[traviscross]

@rustbot labels -I-lang-nominated

We discussed this today in triage, and our general feeling is that conservatively giving a hard error is our preference.

We talked about how, in defense of allowing these attributes liberally, that strictly speaking this is just an optimization, and so an unheeded attribute doesn't affect things in a language specification sense. But we still preferred to just give an error for now.

We note that this is still unstable. This is OK to go forward as far as lang is concerned.

[pnkfelix]

I'll just jot a follow-up note here that one place where we probably do want to continue accepting #[optimize] is on async blocks, with the semantics that the attribute is applied to the poll method of the generated future.

(This is based on lang-team discussion here.)

[Borgerr]

@compiler-errors @nikomatsakis @pnkfelix
Incredibly silly and embarrassing that I accidentally closed this PR, however, my latest commits should reflect the desired changes. The PR can be reopened or I can open this elsewhere.

EDIT: FWIW, I have a different open PR (#131814) for this issue in case this helps speed things up. Thank you for your patience.