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

New compilation error with #[repr(C, align(16))] and bytemuck_derive 1.2.0 #127

Closed
kpreid opened this issue Aug 15, 2022 · 5 comments · Fixed by #128
Closed

New compilation error with #[repr(C, align(16))] and bytemuck_derive 1.2.0 #127

kpreid opened this issue Aug 15, 2022 · 5 comments · Fixed by #128

Comments

@kpreid
Copy link
Sponsor

kpreid commented Aug 15, 2022

The following minimal reproduction case compiles when using bytemuck_derive 1.1.0 but not when using 1.2.0:

#[derive(Copy, Clone, bytemuck::Pod, bytemuck::Zeroable)]
#[repr(C, align(16))]
struct Foo {}

Pod cannot have padding, of course; the intent of this align repr in my real code is not to add padding, but to require the struct fields to add up to a multiple of 16 bytes or fail compilation. This used to work, but now fails to compile with bytemuck_derive 1.2.0:

error: unrecognized representation hint
 --> src/lib.rs:2:16
  |
2 | #[repr(C, align(16))]
  |                ^

I imagine commit 2c97676 is probably responsible for this regression since it substantially changed repr handling.

@Lokathor
Copy link
Owner

What does rust report that the size of Foo is? because, if it's repr(C) then i'm not sure it's going to be size 0 (C is weird about size 0 stuff), and if it's not size 0 and it's align 16 then it will have padding.

@kpreid
Copy link
Sponsor Author

kpreid commented Aug 15, 2022

Sorry, the 0-element struct was just for minimality. The same behavior appears when the struct is non-empty. For example I have the explicitly-"padded" struct

#[repr(C, align(16))]
#[derive(Debug, Copy, Clone, bytemuck::Pod, bytemuck::Zeroable)]
pub(crate) struct PaddedVec3 {
    pub data: [f32; 3],
    padding: f32,
}

that fails in the same way. (4 × f32 = 16 bytes exactly.) bytemuck is still (as far as I know) correctly rejecting padding; the problem here is (from what I can tell) purely a syntactic one: that it is refusing to accept repr(C, align(16)) even when that does not introduce any padding.

@Lokathor
Copy link
Owner

Yeah, this would absolutely be a bug then. I also suspect the cleanup. I'll pass this along to the person that did that cleanup pass, and try to get something soon.

In the mean time, of course you can always manually do the impl (though that's less than ideal i know).

@kpreid
Copy link
Sponsor Author

kpreid commented Aug 20, 2022

FYI, I can confirm the published fix is working. Thanks!

@Lokathor
Copy link
Owner

Excellent!

leod pushed a commit to leod/bytemuck that referenced this issue Jun 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants