Emit an error if #[optimize] is applied to an incompatible item#128458
Merged
Conversation
Collaborator
|
r? @fee1-dead rustbot has assigned @fee1-dead. Use |
9fb7690 to
10cc993
Compare
This comment has been minimized.
This comment has been minimized.
10cc993 to
79a15fa
Compare
79a15fa to
6d7bb12
Compare
compiler-errors
approved these changes
Aug 1, 2024
Contributor
|
@bors r+ rollup |
Collaborator
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
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#54882
The RFC specifies that this should emit a lint. I used the same allow logic as the
coverageattribute (also allowing modules and impl blocks) - this should possibly be changed depending on if it's decided to allow 'propogation' of the attribute.