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(align(8))] is not accepted for arrays #83595

Closed
manuthambi opened this issue Mar 28, 2021 · 8 comments
Closed

#[repr(align(8))] is not accepted for arrays #83595

manuthambi opened this issue Mar 28, 2021 · 8 comments
Labels
A-attributes Area: #[attributes(..)] A-diagnostics Area: Messages for errors, warnings, and lints regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Milestone

Comments

@manuthambi
Copy link

manuthambi commented Mar 28, 2021

Code

struct Data {
    #[repr(align(8))]
    x: [usize; 20]
}

#[repr(align)] was accepted for arrays in rust v1.50, but is rejected in v1.51.

error[E0517]: attribute should be applied to a struct, enum, or union
 --> src/main.rs:3:12
  |
3 |     #[repr(align(8))]
  |            ^^^^^^^^
4 |     x: [usize; 20]
  |     -------------- not a struct, enum, or union

I am not entirely sure whether the attribute actually did the alignment in 1.50, or the compiler just ignored it. Regardless, is this expected behavior, and if it is how do I align an array?

@rustbot rustbot added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Mar 28, 2021
@ehuss
Copy link
Contributor

ehuss commented Mar 28, 2021

Extra validation was added for situations where attributes were ignored (in this case it was #80641). Previous versions issued an unused-attribute warning.

If you want to align an array, I think you can wrap it in a struct with #[repr(align(8))] struct Arr([usize; 20]).

@camelid camelid added A-attributes Area: #[attributes(..)] T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-diagnostics Area: Messages for errors, warnings, and lints labels Mar 28, 2021
@camelid
Copy link
Member

camelid commented Mar 28, 2021

Nominating to see if this was an intended "lint -> hard error" change.

(If there's another way to ask if a change is intended that people on T-compiler prefer, please let me know :)

@rustbot label: +I-nominated

@rustbot

This comment has been minimized.

@rustbot

This comment has been minimized.

@lcnr lcnr added T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 28, 2021
@manuthambi
Copy link
Author

Extra validation was added for situations where attributes were ignored (in this case it was #80641). Previous versions issued an unused-attribute warning.

1.50 didn't warn I think. I haven't specifically configured unusued_attributes in my project. Anyways, it is a good idea to lint/error for unused attributes.

If you want to align an array, I think you can wrap it in a struct with #[repr(align(8))] struct Arr([usize; 20]).

That works, if the compiler guarantees that there will not be any padding at the beginning of Arr struct. Does it?

@scottmcm
Copy link
Member

I am not entirely sure whether the attribute actually did the alignment in 1.50, or the compiler just ignored it.

It just ignored it: https://rust.godbolt.org/z/4rso564PP

(Note also the warning for "#[warn(unused_attributes)] on by default".)

It's my understanding that this is known and expected fallout from fixing compiler bugs where it had previously forgotten to run validation in certain positions.

@Mark-Simulacrum Mark-Simulacrum modified the milestones: 1.52.0, 1.51.0 Apr 27, 2021
@scottmcm
Copy link
Member

We discussed this in the lang team meeting today.

This is an intentional bugfix to resolve a place where attributes weren't being checked but were supposed to have been. Note that this never did anything, so any unsafe code that was expecting this to align things is potentially unsound.

(There have been a few of these, recently -- for example 1.49 fixed this on enum variants.)


repr(Rust) has very few guarantees. If you strictly need no padding at the beginning, you can use #[repr(C, align(8))] on the struct: https://rust.godbolt.org/z/x6648cWrE

@camelid camelid removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Apr 27, 2021
@camelid
Copy link
Member

camelid commented Apr 27, 2021

Should I-nominated be removed now that the issue is closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Area: #[attributes(..)] A-diagnostics Area: Messages for errors, warnings, and lints regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants