feat: add a 'records' example, and some required instructions#116
feat: add a 'records' example, and some required instructions#116sd2k wants to merge 5 commits intoarcjet:mainfrom
Conversation
Prior to this commit there was ostensibly no support for 'record' WIT types, but there wasn't actually much required for them to work. This commit adds an example for use in the test harness and the required instructions to load/store integer record fields. Relates to #4.
| quote_in! { self.body => | ||
| $['\r'] | ||
| $result := $WAZERO_API_DECODE_U32($operand) | ||
| $result := $WAZERO_API_DECODE_U32(uint64($operand)) |
There was a problem hiding this comment.
This may look weird but operand can sometimes be a uint32 now, if this U32FromI32 instruction is called after a PointerLoad, as happens in records, which calls ReadUint32Le. Since DecodeU32 requires the arg to be a uint64, we have to explicitly cast, which should be a no-op since the u32 always fits in the u64.
There was a problem hiding this comment.
I've actually encountered this today and it is a bigger problem than just this fix. I think all of the read/writes need to be converted to the uint64 variant but that large of a change is going to need validation.
blaine-arcjet
left a comment
blaine-arcjet
left a comment
There was a problem hiding this comment.
Sorry for the delay in reviewing. I have some thoughts inline but I'm happy to apply them so we can land this.
| let operand = &operands[0]; | ||
| quote_in! { self.body => | ||
| $['\r'] | ||
| $(&value) := int64($operand) |
There was a problem hiding this comment.
Is there a Wazero utility for this since a U64 could wrap in an I64?
There was a problem hiding this comment.
Not that I can see 😕 from what I can tell the behaviour is correct though and follows two's complement behaviour that both Go and Wasm expect.
examples/records/wit/records.wit
Outdated
| } | ||
|
|
||
| world records { | ||
| use types.{foo}; |
There was a problem hiding this comment.
If the use is avoided, does this skip the interface generation?
There was a problem hiding this comment.
It would be invalid WIT:
error: failed to resolve directory while parsing WIT for path [/home/ben/repos/rust/gravity/examples/records/wit]
Caused by:
failed to parse package: /home/ben/repos/rust/gravity/examples/records/wit
Caused by:
name `foo` is not defined
--> /home/ben/repos/rust/gravity/examples/records/wit/records.wit:16:30
|
16 | export modify-foo: func(f: foo) -> foo;
| ^--
we could declare the foo type inline though if we want to avoid the interface generation, I've done that in 8544864 👍
Co-authored-by: blaine-arcjet <146491715+blaine-arcjet@users.noreply.github.com>
2 participants
Prior to this commit there was ostensibly no support for 'record' WIT
types, but there wasn't actually much required for them to work. This
commit adds an example for use in the test harness and the required
instructions to load/store integer record fields.
Relates to #4.