Closed Bug 1384723 Opened 2 years ago Closed 2 years ago

[wasm] Remove experimental WebAssembly text support from the SM

Categories

(Core :: JavaScript Engine: JIT, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: yury, Assigned: yury)

Details

Attachments

(1 file)

Two things happened:

1. there is a spec for WebAssembly text format -- https://webassembly.github.io/spec/text/index.html
2. the SM can send binary code to the devtools and text transformation is performed there (see bug 1364535)

We can remove format introduced in bug 1274618 from SpiderMonkey code, and in the future, we can add this or similar text transform on devtools side if we decide to do that.
Assignee: nobody → ydelendik
Comment on attachment 8890551 [details]
Bug 1384723 - Remove experimental WebAssembly text support.

https://reviewboard.mozilla.org/r/161692/#review168396

::: js/src/builtin/TestingFunctions.cpp:612
(Diff revision 1)
> -            return false;
> -        experimental = match;
> -    }
> -
>      StringBuffer buffer(cx);
> -    bool ok;
> +    bool ok = wasm::BinaryToText(cx, bytes, length, buffer);

The old code would return false if an extra argument is present and anything other than "experimental". It may be worthwhile to retain part of that logic, like this:

if (args.length() > 1) {
  return false;
}

to leave open the possibility of future expansion.
Attachment #8890551 - Flags: review?(sunfish) → review+
Pushed by ydelendik@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a09638bf8e7a
Remove experimental WebAssembly text support. r=sunfish
https://hg.mozilla.org/mozilla-central/rev/a09638bf8e7a
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.