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

fix compile error on the std.c.EXC.MASK field on darwin #21273

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

Conversation

srijan-paul
Copy link

@srijan-paul srijan-paul commented Sep 1, 2024

The backing integer for the MASK struct is a u32, but the bool fields sum up to a u13.
I've added 19 bits of padding.

Alternatively, we could add 3 bits of padding and make it a u16 backed packed struct?
EDIT: Looks using std.c.darwing.exception_mask_t would be a better idea here.
Is there a reason why the std library doesn't use C types in these places?

Reference: #21094

P.S: Should we consider having a test block for this just to see if it compiles? Something like:

test {
   std.testing.expectEqual(32, @bitSizeOf(MASK));
}

@srijan-paul srijan-paul changed the title fix the std.c.EXC.MASK field on darwin fix compile error on the std.c.EXC.MASK field on darwin Sep 1, 2024
@FnControlOption
Copy link
Contributor

Alternatively, we could add 3 bits of padding and make it a u16 backed packed struct?

I would use exception_mask_t instead (or whichever type has the same size as c_uint)

Reference: https://github.com/apple-oss-distributions/xnu/blob/main/osfmk/mach/exception_types.h#L207

@srijan-paul
Copy link
Author

or whichever type has the same size as c_uint

In that case, why not use c_uint directly?

@FnControlOption
Copy link
Contributor

exception_mask_t is already defined as c_uint in std.c.darwin, but packed structs in the standard library don’t use C types for some reason

@srijan-paul
Copy link
Author

Interesting. I wonder why that decision was taken.
I could change the u32 to exception_mask_t in this PR.

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 this pull request may close these issues.

2 participants