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

disallow repr() on invalid items #133925

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

folkertdev
Copy link
Contributor

@folkertdev folkertdev commented Dec 5, 2024

fixes #129606
fixes #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 #132391 and generally has been working on attributes

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 enables the align attribute on trait methods.
@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 Dec 5, 2024
@compiler-errors
Copy link
Member

@bors try

@compiler-errors
Copy link
Member

Nominating as a courtesy for T-lang, should be a trivial decision since this was likely never intended

@compiler-errors compiler-errors added the I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy; this does not imply nomination label Dec 5, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request 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
Copy link
Contributor

bors commented Dec 5, 2024

⌛ Trying commit dfd76c1 with merge 617b810...

@rust-log-analyzer

This comment has been minimized.

@jdonszelmann
Copy link
Contributor

Oh nice! If this fixes an issue you have right now, great! Especially the tests are nice so I can't mess it up again, but the code in check_attrs.rs will likely be deleted within a month. (sorry)

@folkertdev
Copy link
Contributor Author

well this change unintentionally turns out to fix that ICE. And yeah the test coverage is the most important part, I'm not at all attached to this code. Good luck with your rebase!

@jdonszelmann
Copy link
Contributor

jdonszelmann commented Dec 5, 2024

lol thanks, can't be much worse than the current one (I'm 4 days in)

@compiler-errors
Copy link
Member

r=me when T-lang comes back w/ a decision

@rustbot team

@compiler-errors compiler-errors added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). T-lang Relevant to the language team, which will review and decide on the PR/issue. I-lang-nominated Nominated for discussion during a lang team meeting. and removed 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 Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy; this does not imply nomination I-lang-nominated Nominated for discussion during a lang team meeting. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ICE: malformed malformed repr(align(N)) #[repr()] is allowed where it shouldn't
6 participants