Fix wasm-module-builder.js

[thibaudmichaud]

See l.1166, the type property for imported functions should be named "type_index".

[backes]

LGTM, this is also in line with the upstream file in V8 (even though it's formatted a bit differently there).

[sbc100]

Is this change related to memory64 or should we land it upstream?

[thibaudmichaud]

The base spec calls the property "type", but it is consistent about it, so it does not need fixing.

[sbc100]

The base spec calls the property "type", but it is consistent about it, so it does not need fixing.

Sorry, I must be missing something. Why does this need fixing here then?

[thibaudmichaud]

I don't know why the scripts have diverged, but I noticed that they did because of test failures in V8:

memory64:

this.imports.push({module: module, name: name, kind: kExternalFunction,
                       type: type_index});
// ...
section.emit_u32v(imp.type_index);

main spec:

this.imports.push({module: module, name: name, kind: kExternalFunction,
                       type: type_index});
// ...
section.emit_u32v(imp.type);

[sbc100]

The memory64 proposal is based on the upstream wasm-3.0 branch and I don't see any divergence in this file. Perhaps its the upstream/wasm-3.0 branch that needs fixing:

$ git diff upstream/wasm-3.0 test/js-api/wasm-module-builder.js

BTW, it looks like the upstream/main branch also has one occurrence of type_index:

$ git grep section.emit_u32v.*type_index upstream/main
upstream/main:test/js-api/wasm-module-builder.js:          section.emit_u32v(func.type_index);
$ git grep section.emit_u32v.*type_index upstream/wasm-3.0
upstream/wasm-3.0:test/js-api/wasm-module-builder.js:            section.emit_u32v(imp.type_index);
upstream/wasm-3.0:test/js-api/wasm-module-builder.js:          section.emit_u32v(func.type_index);

[thibaudmichaud]

Ack, I was looking at the main branch (which must be the one we use to run the spec tests in V8).
It does look like this comes from wasm-3.0, I'll reopen the PR there.