-
Notifications
You must be signed in to change notification settings - Fork 452
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
Update JavaScript WasmModuleBuilder with new functionality. #530
Conversation
test/harness/wasm-module-builder.js
Outdated
@@ -100,17 +107,23 @@ class WasmFunctionBuilder { | |||
|
|||
addBody(body) { | |||
for (let b of body) { | |||
if (typeof b !== 'number' || (b & (~0xFF)) !== 0 ) |
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.
It seems that in this case, the version in the spec repo was updated to do even stricter tests. Hence we should merge this change back to v8.
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.
Good catch. I had just blindly copied over the v8 version instead of merging the two.
test/harness/wasm-module-builder.js
Outdated
@@ -365,10 +378,9 @@ class WasmModuleBuilder { | |||
if (debug) print("emitting memory @ " + binary.length); | |||
binary.emit_section(kMemorySectionCode, section => { | |||
section.emit_u8(1); // one memory entry | |||
const has_max = wasm.memory.max !== undefined; |
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.
Also here, the spec version looks more reasonable than the v8 version.
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.
Done.
This is ready to merge. |
lgtm |
Fix typing of `ref.i31` in the appendix
The original version of this file was a fork of the V8 module builder. This is just an update with the additional functionality that we have seen fit to add in our tests, including, e.g., the new names section format.