-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
support variant, enum, and union derives #4359
support variant, enum, and union derives #4359
Conversation
This is the second stage of implementing bytecodealliance#4308. It adds support for deriving variant, enum, and union impls for `ComponentType`, `Lift`, and `Lower`. It also fixes derived record impls for generic `struct`s, which I had intended to support in my previous commit, but forgot to test. Signed-off-by: Joel Dice <joel.dice@fermyon.com>
Subscribe to Label Actioncc @peterhuene
This issue or pull request has been labeled: "wasmtime:api"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
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.
This looks fantastic to me, thanks for this! I've got one question about the u8 limit, but if it's what I suspect (just more effort needed) mind filing a follow-up issue? I think it's ok to leave as-is for awhile since we're unlikely to get >256 variant-things any time in the near future.
input.ident.span(), | ||
"`enum`s with more than 256 variants not yet supported", | ||
) | ||
})?; |
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.
Is this mostly to just simplify this for now where you don't have to worry about 16-vs-8 bit loads?
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.
Yeah, I was just keeping it simple to start with. I was also not sure what the theoretical max was for the component model. 2^32? More?
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.
The Canonical ABI defines that in the discriminant_type
function, and the current maximum is 2^32.
Thanks to @jameysharp for the suggestion! Signed-off-by: Joel Dice <joel.dice@fermyon.com>
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.
I love it. @alexcrichton, want to merge this?
This is the second stage of implementing #4308. It adds support for deriving
variant, enum, and union impls for
ComponentType
,Lift
, andLower
. Italso fixes derived record impls for generic
struct
s, which I had intended tosupport in my previous commit, but forgot to test.
Signed-off-by: Joel Dice joel.dice@fermyon.com