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

Attempt to fix the packed struct problem #3745

Closed
wants to merge 3 commits into from
Closed

Attempt to fix the packed struct problem #3745

wants to merge 3 commits into from

Conversation

ikskuh
Copy link
Contributor

@ikskuh ikskuh commented Nov 23, 2019

Attempt to fix the packed struct problem (#2627, #3651) by removing ABI alignment in packed structs.

This crashes right now with an assertion in analyze.zig:9051 as the packed struct is smaller than it's alignment.

The code was still tested with bin/zig build test -Dskip-release -Dskip-libc -Dskip-non-native, but without test/stage1/behaviour/struct.zig (which introduces the crash)

@ikskuh
Copy link
Contributor Author

ikskuh commented Nov 23, 2019

I'm narrowing down the code in the test suite struct.zig where the assertion happens. To my understanding this can only happen for packed structs which meet some alignment rules regarding to 3 byte (for all other cases the alignment should fit)

@ikskuh
Copy link
Contributor Author

ikskuh commented Nov 23, 2019

Okay, only two tests break with this pull request.

@ikskuh
Copy link
Contributor Author

ikskuh commented Nov 25, 2019

Is already said in IRC, i withdraw this pull request, but leave it open for you to merge the tests into master.

@andrewrk
Copy link
Member

andrewrk commented Jan 2, 2020

I added the test cases to #3133 (comment) and will be sure to cover them.

Apologies for the delay in getting to this issue. I do have #3133 and associated bug reports marked as high priority for this release cycle. I know it's particularly important for your use cases.

@andrewrk andrewrk closed this Jan 2, 2020
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