Skip to content
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

#[repr()] is allowed where it shouldn't #129606

Closed
Kyuuhachi opened this issue Aug 26, 2024 · 4 comments · Fixed by #133925
Closed

#[repr()] is allowed where it shouldn't #129606

Kyuuhachi opened this issue Aug 26, 2024 · 4 comments · Fixed by #133925
Assignees
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-repr Area: the `#[repr(stuff)]` attribute C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Kyuuhachi
Copy link
Contributor

I tried this code:

#[repr()] const CONST: u32 = 8;
#[repr()] struct Struct;
#[repr()] trait Trait {}
#[repr()] impl Trait for Struct {}
#[repr()] fn func() { #[repr()] let a = 4; }

I expected to see this happen: #[repr] doesn't make sense on other than struct/enum/union, so it shouldn't be permitted. Every case except the struct should be an error. Not sure if that one ought to be legal either, but I guess the current warning is enough there.

Instead, this happened: Plenty of unused attribute warnings, but it compiles.

Meta

Tested on playground, both 1.80.1 and 1.82.0-nightly (2024-08-24 f167efad2f51088d8618).

@Kyuuhachi Kyuuhachi added the C-bug Category: This is a bug. label Aug 26, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 26, 2024
@jieyouxu
Copy link
Member

jieyouxu commented Aug 26, 2024

cc #128444 (which is copied over to #128581)

@jieyouxu jieyouxu added A-attributes Area: Attributes (`#[…]`, `#![…]`) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 26, 2024
@jieyouxu
Copy link
Member

If someone wants to fix this they should check if there's an RFC for #[repr] attribute and any follow-up T-lang discussions/decisions. The PR should also probably also be nominated for T-lang signoff.

@jieyouxu jieyouxu added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Aug 26, 2024
@compiler-errors
Copy link
Member

I'll fold this into #129422 (or probably just do it as a follow-up)

@compiler-errors compiler-errors self-assigned this Aug 26, 2024
@compiler-errors
Copy link
Member

If someone wants to fix this they should check if there's an RFC for #[repr] attribute and any follow-up T-lang discussions/decisions. The PR should also probably also be nominated for T-lang signoff.

#[repr()] on functions should be gated behind fn_align: #82232

Otherwise #[repr()] should only be allowed on ADTs.

@workingjubilee workingjubilee added the A-repr Area: the `#[repr(stuff)]` attribute label Nov 1, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Dec 5, 2024
…<try>

disallow `repr()` on invalid items

fixes rust-lang#129606

Disallows `repr()` (so a repr with no arguments) on items where that won't ever make sense.

Also this generates an error when `repr` is used on a trait method and the `fn_align` feature is not enabled. Looks like that was missed here:

https://github.com/rust-lang/rust/pull/110313/files

Which first accepts the align attribute on trait methods.

r? `@compiler-errors`
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 18, 2025
…<try>

disallow `repr()` on invalid items

fixes rust-lang#129606
fixes rust-lang#132391

Disallows `repr()` (so a repr with no arguments) on items where that won't ever make sense.

Also this generates an error when `repr` is used on a trait method and the `fn_align` feature is not enabled. Looks like that was missed here:

https://github.com/rust-lang/rust/pull/110313/files

Which first accepts the align attribute on trait methods.

r? `@compiler-errors`

cc `@jdonszelmann` who claimed rust-lang#132391 and generally has been working on attributes
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Feb 6, 2025
…r=compiler-errors

disallow `repr()` on invalid items

fixes rust-lang#129606
fixes rust-lang#132391

Disallows `repr()` (so a repr with no arguments) on items where that won't ever make sense.

Also this generates an error when `repr` is used on a trait method and the `fn_align` feature is not enabled. Looks like that was missed here:

https://github.com/rust-lang/rust/pull/110313/files

Which first accepts the align attribute on trait methods.

r? `@compiler-errors`

cc `@jdonszelmann` who claimed rust-lang#132391 and generally has been working on attributes
@bors bors closed this as completed in 4b7e55a Feb 7, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 7, 2025
Rollup merge of rust-lang#133925 - folkertdev:improve-repr-warnings, r=compiler-errors

disallow `repr()` on invalid items

fixes rust-lang#129606
fixes rust-lang#132391

Disallows `repr()` (so a repr with no arguments) on items where that won't ever make sense.

Also this generates an error when `repr` is used on a trait method and the `fn_align` feature is not enabled. Looks like that was missed here:

https://github.com/rust-lang/rust/pull/110313/files

Which first accepts the align attribute on trait methods.

r? `@compiler-errors`

cc `@jdonszelmann` who claimed rust-lang#132391 and generally has been working on attributes
@chenyukang chenyukang reopened this Feb 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: Attributes (`#[…]`, `#![…]`) A-repr Area: the `#[repr(stuff)]` attribute C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants
@chenyukang @Kyuuhachi @compiler-errors @jieyouxu @workingjubilee @rustbot and others