Skip to content

Emit an error if #[optimize] is applied to an incompatible item#128458

Merged
bors merged 1 commit into
rust-lang:masterfrom
clubby789:optimize-unused-attr
Aug 1, 2024
Merged

Emit an error if #[optimize] is applied to an incompatible item#128458
bors merged 1 commit into
rust-lang:masterfrom
clubby789:optimize-unused-attr

Conversation

@clubby789
Copy link
Copy Markdown
Contributor

#54882

The RFC specifies that this should emit a lint. I used the same allow logic as the coverage attribute (also allowing modules and impl blocks) - this should possibly be changed depending on if it's decided to allow 'propogation' of the attribute.

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Jul 31, 2024

r? @fee1-dead

rustbot has assigned @fee1-dead.
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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 31, 2024
Copy link
Copy Markdown
Contributor

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a test

@rust-cloud-vms rust-cloud-vms Bot force-pushed the optimize-unused-attr branch 2 times, most recently from 9fb7690 to 10cc993 Compare July 31, 2024 20:14
@rust-log-analyzer

This comment has been minimized.

@rust-cloud-vms rust-cloud-vms Bot force-pushed the optimize-unused-attr branch from 10cc993 to 79a15fa Compare July 31, 2024 20:26
@rust-cloud-vms rust-cloud-vms Bot force-pushed the optimize-unused-attr branch from 79a15fa to 6d7bb12 Compare July 31, 2024 20:28
@compiler-errors
Copy link
Copy Markdown
Contributor

@bors r+ rollup

@bors
Copy link
Copy Markdown
Collaborator

bors commented Aug 1, 2024

📌 Commit 6d7bb12 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 1, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 1, 2024
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#128416 (android: Remove libstd hacks for unsupported Android APIs)
 - rust-lang#128437 (improve bootstrap to allow selecting llvm tools individually)
 - rust-lang#128450 (Create COFF archives for non-LLVM backends)
 - rust-lang#128458 (Emit an error if `#[optimize]` is applied to an incompatible item)
 - rust-lang#128477 (Finish blessing `coverage/mcdc` tests after LLVM 19 upgrade)
 - rust-lang#128478 (Ignore `use` declaration reformatting in `.git-blame-ignore-revs`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7c60525 into rust-lang:master Aug 1, 2024
@rustbot rustbot added this to the 1.82.0 milestone Aug 1, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 1, 2024
Rollup merge of rust-lang#128458 - clubby789:optimize-unused-attr, r=compiler-errors

Emit an error if `#[optimize]` is applied to an incompatible item

rust-lang#54882

The RFC specifies that this should emit a lint. I used the same allow logic as the `coverage` attribute (also allowing modules and impl blocks) - this should possibly be changed depending on if it's decided to allow 'propogation' of the attribute.
tgross35 added a commit to tgross35/rust that referenced this pull request Aug 8, 2024
…pos, r=BoxyUwU

Emit an error for invalid use of the `#[no_sanitize]` attribute

fixes rust-lang#128487.

Currently, the use of the `#[no_sanitize]` attribute for Mod, Impl,... is incorrectly permitted. This PR will correct this issue by generating errors, and I've also added some UI test cases for it.

Referenced rust-lang#128458. As far as I know, the `#[no_sanitize]` attribute can only be used with functions, so I changed that part to `Fn` and `Method` using `check_applied_to_fn_or_method`. However, I couldn't find explicit documentation on this, so I could be mistaken...
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 8, 2024
Rollup merge of rust-lang#128552 - s7tya:check-no-sanitize-attribute-pos, r=BoxyUwU

Emit an error for invalid use of the `#[no_sanitize]` attribute

fixes rust-lang#128487.

Currently, the use of the `#[no_sanitize]` attribute for Mod, Impl,... is incorrectly permitted. This PR will correct this issue by generating errors, and I've also added some UI test cases for it.

Referenced rust-lang#128458. As far as I know, the `#[no_sanitize]` attribute can only be used with functions, so I changed that part to `Fn` and `Method` using `check_applied_to_fn_or_method`. However, I couldn't find explicit documentation on this, so I could be mistaken...
veluca93 added a commit to veluca93/rust that referenced this pull request Jun 1, 2026
This commit stabilizes the `#[optimize]` attribute, which allows for more fine-grained control
of optimization levels.

Tracking issue: rust-lang#54882
Reference PRs: rust-lang/reference#2278

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

> Describe each behavior being stabilized and give a short example of code that will now be accepted.

```rust
// Instructs the optimization pipeline to prioritize smaller code size over speed.
pub fn binary_size_sensitive() {
    // ...
}

// Instructs the optimization pipeline to prioritize execution speed over code size.
pub fn performance_critical() {
    // ...
}

// Disables optimizations entirely for this item.
pub fn debug_only() {
    // ...
}
```

We might want to allow specifying a per-function optimization level (i.e. `#[optimize(level = 3)]`).
To my understanding, backend support for this is not quite there yet (and this - or other -
extensions have consequently not been implemented yet).

Applying the attribute on a module level is also not implemented yet.

- Description of the new attribute: rust-lang/reference#2278

- https://rust-lang.github.io/rfcs/2412-optimize-attr.html

- Should we also implement optimize(always)? optimize(level=x)?

Not yet.

- Should there be any way to specify what global optimization for speed level is used in conjunction with the optimization for speed option (e.g. -Copt-level=s3 could be equivalent to -Copt-level=3 and #[optimize(size)] on the crate item).

Not yet.

The `optimize(none)` variant was added. The variants above were discussed here: rust-lang#54882 (comment)

- `#[optimize(none)]` implies `#[inline(never)]` to be effective (rust-lang#136329)
- As with all optimization-controlling flags, the effects on speed and binary size can be hard to predict: rust-lang#54882 (comment)
- Applying the attribute to incompatible items emits an error: rust-lang#128458

The standard library itself uses this attribute in a few places

- Wiring it up to the llvm codegen backend: https://github.com/rust-lang/rust/blob/c0bb140a37c81cf59a0b40c21c9413059644e294/compiler/rustc_codegen_llvm/src/attributes.rs#L416
- Catching usage on incompatible items: rust-lang#128458

- Basic tests: https://github.com/rust-lang/rust/blob/c0bb140a37c81cf59a0b40c21c9413059644e294/tests/ui/attributes/optimize.rs
- Testing the effect on compilation: https://github.com/rust-lang/rust/blob/c0bb140a37c81cf59a0b40c21c9413059644e294/tests/codegen-llvm/optimize-attr-1.rs

Trivial changes to rust-analyzer to support the new attribute as an inert attribute
(part of this PR).

As far as the AM is concerned, this attribute doesn't exist.

Most of the work was not done by me, I'm just writing the stabilization report :-)

@nagisa did the initial implementation, @clubby789 implemented optimize(none) and
fixed the attribute being allowed in too many places.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants