-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
sema: add union alignment resolution #17658
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
Conversation
c906f37
to
1e6ecf8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actual logic looks good, a few minor comments.
1e6ecf8
to
ac52cb9
Compare
- Add resolveUnionAlignment, to resolve a union's alignment only, without triggering layout resolution. - Update resolveUnionLayout to cache size, alignment, and padding. abiSizeAdvanced and abiAlignmentAdvanced now use this information instead of computing it each time.
… guess was correct
ac52cb9
to
0574643
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! I'd be happy to merge this, but I'll give it a couple days in case Andrew has any comments I've missed.
@mlugg please prefer a |
Ah, okay, my bad! |
This is just the union alignment changes extracted from #17490.
Changes:
resolveUnionAlignment
, to resolve a union's alignment only, without triggering layout resolution.resolveUnionLayout
to cache size, alignment, and padding.abiSizeAdvanced
andabiAlignmentAdvanced
now use this information instead of computing it each time.