Comments on wasm-module-builder.js

[Ms2ger]

In https://bugzilla.mozilla.org/show_bug.cgi?id=1500442, @bnjbvr suggests:

::: testing/web-platform/tests/wasm/jsapi/wasm-module-builder.js
@@ +134,5 @@
> +  }
> +
> +  addLocals(locals, names) {
> +    const old_num_locals = this.getNumLocals();
> +    if (!this.locals) this.locals = []

Wouldn't it be better to have this.locals = [] in the ctor?

@@ +137,5 @@
> +    const old_num_locals = this.getNumLocals();
> +    if (!this.locals) this.locals = []
> +    this.locals.push(locals);
> +    if (names) {
> +      if (!this.local_names) this.local_names = [];

ditto

@@ +243,5 @@
>                         type: type_index});
>      return this.num_imported_funcs++;
>    }
>  
> +  addImportedGlobal(module = "", name, type, mutable = false) {

Uh, mixing optional params with non-optional always makes me a bit nervous...

Maybe @hammacher would like to respond or address the comments in the V8 repo?

[backes]

Thanks for the comments, looks reasonable. Fixing in this CL: https://crrev.com/c/1301476