Skip to content
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

Merged
merged 3 commits into from
Jul 27, 2017

Conversation

titzer
Copy link
Contributor

@titzer titzer commented Jul 26, 2017

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.

@@ -100,17 +107,23 @@ class WasmFunctionBuilder {

addBody(body) {
for (let b of body) {
if (typeof b !== 'number' || (b & (~0xFF)) !== 0 )
Copy link
Member

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.

Copy link
Contributor Author

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.

@@ -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;
Copy link
Member

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@titzer
Copy link
Contributor Author

titzer commented Jul 27, 2017

This is ready to merge.

@gahaas
Copy link
Collaborator

gahaas commented Jul 27, 2017

lgtm

@titzer titzer merged commit fe1abd0 into master Jul 27, 2017
@jfbastien jfbastien deleted the update_wasm_module_builder branch July 27, 2017 16:17
ngzhian added a commit to ngzhian/spec that referenced this pull request Nov 16, 2021
rossberg added a commit that referenced this pull request Apr 21, 2024
Fix typing of `ref.i31` in the appendix
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.

3 participants