Split out the flags type from record#209
Merged
alexcrichton merged 4 commits intobytecodealliance:mainfrom Apr 28, 2022
Merged
Conversation
This commit creates a dedicated `flags` type which is distinct from the `record` type rather than the previous inferred-from-the-structure logic. This also additionally changes the canonical ABI of the `flags` type where 64-bit flags are now passed as two `i32` values instead of one `i64`. This ended up changing a significant amount of the logic internally in each code generator, notably around the new lift/lower behavior. Along the way I tried to refactor code to support 64+ flags in a few more places. While some support may be there, though, this is untested and will need a full-fledged feature in the future.
21 tasks
peterhuene
approved these changes
Apr 28, 2022
willemneal
pushed a commit
to theahaco/wit-bindgen
that referenced
this pull request
May 31, 2022
* Split out the `flags` type from `record` This commit creates a dedicated `flags` type which is distinct from the `record` type rather than the previous inferred-from-the-structure logic. This also additionally changes the canonical ABI of the `flags` type where 64-bit flags are now passed as two `i32` values instead of one `i64`. This ended up changing a significant amount of the logic internally in each code generator, notably around the new lift/lower behavior. Along the way I tried to refactor code to support 64+ flags in a few more places. While some support may be there, though, this is untested and will need a full-fledged feature in the future. * Fix a test * Fix some more flags * Fix more tests
|
what was the rationale for splitting a 64 bitflag into two fields? As you noticed in refactoring, this has a lot of impact and isn't universally desirable. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit creates a dedicated
flagstype which is distinct from therecordtype rather than the previous inferred-from-the-structurelogic. This also additionally changes the canonical ABI of the
flagstype where 64-bit flags are now passed as two
i32values instead ofone
i64. This ended up changing a significant amount of the logicinternally in each code generator, notably around the new lift/lower
behavior.
Along the way I tried to refactor code to support 64+ flags in a few
more places. While some support may be there, though, this is untested
and will need a full-fledged feature in the future.