Skip to content

port to zig 0.16.0#242

Merged
malcolmstill merged 1 commit into
malcolmstill:masterfrom
dasimmet:zig-0.16.x
Jun 21, 2026
Merged

port to zig 0.16.0#242
malcolmstill merged 1 commit into
malcolmstill:masterfrom
dasimmet:zig-0.16.x

Conversation

@dasimmet

@dasimmet dasimmet commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

seems to work for me so far, but i had to vendor the new standard library std.Io.Reader.takeLeb128
to make it not read extra bytes to pass the test suite.
also error enums returned from std.Io.Reader.takeEnum changed to error.InvalidEnumTag and i had to adapt testsuite errors to match.

@dasimmet dasimmet marked this pull request as draft June 12, 2026 11:34
@dasimmet dasimmet force-pushed the zig-0.16.x branch 2 times, most recently from 72f915f to 8451a94 Compare June 13, 2026 06:59
@dasimmet dasimmet marked this pull request as ready for review June 13, 2026 09:44
@malcolmstill

Copy link
Copy Markdown
Owner

Thanks for this @dasimmet.

Are you able to explain the LEB128 issue a bit more as you understand it? Would be great not to have to vendor the code so I'm keen to understand if there are any alternatives here before committing to that.

@dasimmet

Copy link
Copy Markdown
Contributor Author

The new implementation of 0.16 takeLeb will happily keep reading bytes from the reader as long as the continuation bit is set due to the final while loop, even if its more than 5 bytes (for 32bit ints):

22dc32f#diff-2bc993f98f63c8cc2df8f9246692eae522dad0cdda50e48e692803979a04662bL95

The wasm spec forbids such "overlong" leb128 values, while technically i think they are valid. Thats what these "integer representation too long" cases test.

I guess we can search or file an issue with zig to see if std behavior is intentional, or check after calling takeleb128 if more bytes were consumed maybe

@dasimmet dasimmet force-pushed the zig-0.16.x branch 5 times, most recently from c892595 to a917832 Compare June 16, 2026 09:39
Comment thread src/module.zig
also works for master (0.17.0-dev.864+3deb86baf)

adapt errors in testrunner cases

adapt errors in testrunner cases
@malcolmstill malcolmstill merged commit 8384227 into malcolmstill:master Jun 21, 2026
13 checks passed
@malcolmstill

Copy link
Copy Markdown
Owner

Thanks @dasimmet

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