Skip to content

allow #[rustfmt::skip] in combination with #[naked] #140626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 4, 2025

Conversation

folkertdev
Copy link
Contributor

@folkertdev folkertdev commented May 3, 2025

fixes #140623

We very deliberately use an allowlist to prevent weird interactions with #[naked], hopefully we've now found all of the useful combinations.

cc @Amanieu

@rustbot
Copy link
Collaborator

rustbot commented May 3, 2025

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 A-attributes Area: Attributes (`#[…]`, `#![…]`) 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 May 3, 2025
@rustbot
Copy link
Collaborator

rustbot commented May 3, 2025

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann

@Amanieu
Copy link
Member

Amanieu commented May 3, 2025

Would it make sense to generally allow all tool/proc_macro attributes and only restrict builtin attributes?

@jdonszelmann
Copy link
Contributor

jdonszelmann commented May 3, 2025

Sometimes those are the same, I would need to recheck what the exact behavior is but these two sets could shadow eachother, if you try

@folkertdev
Copy link
Contributor Author

Yeah I don't seen an immediate way to rule out all builtin attributes. There is this

https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/attr/trait.AttributeExt.html#method.is_proc_macro_attr

@folkertdev
Copy link
Contributor Author

Though I agree that we probably should only disallow builtin attributes and allow user proc macros.

@Amanieu
Copy link
Member

Amanieu commented May 3, 2025

In any case, this is fine for now until we can find a better solution. I think the proper solution moving forward will be to have all function attributes properly handler the case of naked functions.

@bors r+

@bors
Copy link
Collaborator

bors commented May 3, 2025

📌 Commit 9aee0aa has been approved by Amanieu

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 May 3, 2025
@fee1-dead fee1-dead assigned Amanieu and unassigned fee1-dead May 4, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request May 4, 2025
Rollup of 7 pull requests

Successful merges:

 - rust-lang#139675 (Add the AVX10 target features)
 - rust-lang#140286 (Check if format argument is identifier to avoid error err-emit)
 - rust-lang#140456 (Fix test simd/extract-insert-dyn on s390x)
 - rust-lang#140551 (Move some tests out of tests/ui)
 - rust-lang#140588 (Adjust some ui tests re. target-dependent errors)
 - rust-lang#140617 (Report the `unsafe_attr_outside_unsafe` lint at the closest node)
 - rust-lang#140626 (allow `#[rustfmt::skip]` in combination with `#[naked]`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1239f49 into rust-lang:master May 4, 2025
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 4, 2025
Rollup merge of rust-lang#140626 - folkertdev:naked-rustfmt-skip, r=Amanieu

allow `#[rustfmt::skip]` in combination with `#[naked]`

fixes rust-lang#140623

We very deliberately use an allowlist to prevent weird interactions with `#[naked]`, hopefully we've now found all of the useful combinations.

cc `@Amanieu`
@rustbot rustbot added this to the 1.88.0 milestone May 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) 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.

#[rustfmt::skip] attribute incompatible with #[unsafe(naked)]
6 participants