Bug 163699 - WebAssembly API: implement exception constructors properly
Summary: WebAssembly API: implement exception constructors properly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: JF Bastien
URL:
Keywords:
Depends on:
Blocks: 161709 163768
  Show dependency tree / graph
 
Reported: 2016-10-19 15:23 PDT by JF Bastien
Modified: 2016-10-20 18:22 PDT (History)
8 users (show)

See Also:

Attachments
patch (64.16 KB, patch)
2016-10-20 16:55 PDT, JF Bastien 		no flags Details | Formatted Diff | Diff
patch (86.22 KB, patch)
2016-10-20 17:25 PDT, JF Bastien 		keith_miller: review+
keith_miller: commit-queue-
Details | Formatted Diff | Diff
patch (86.23 KB, patch)
2016-10-20 17:42 PDT, JF Bastien 		commit-queue: commit-queue-
Details | Formatted Diff | Diff
patch (86.22 KB, patch)
2016-10-20 17:47 PDT, JF Bastien 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description JF Bastien 2016-10-19 15:23:12 PDT 
The current exception constructors just throw instead of doing The Right Thing.

There's a small quibble about whether just calling the constructor as a function should be a TypeError: https://github.com/WebAssembly/design/issues/825
Comment 1 JF Bastien 2016-10-20 16:55:09 PDT 
Created attachment 292286 [details]
patch
Comment 2 JF Bastien 2016-10-20 16:57:10 PDT 
This was trickier than I thought it was! It's almost there:

I don't think I'm setting the .name property correctly though: if you do String(new WebAssembly.CompileError()) then you get "Error", not "CompileError".

Any idea what I've missed?
Comment 3 JF Bastien 2016-10-20 17:25:28 PDT 
Created attachment 292291 [details]
patch

The previous patch was missing a bunch of `#if ENABLE(WEBASSEMBLY)` which made the 32-bit builds *very* sad. Don't be a jerk: make it happy.
Comment 4 Keith Miller 2016-10-20 17:39:28 PDT 
Comment on attachment 292291 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=292291&action=review

r=me with typo fix.

> JSTests/ChangeLog:9
> +         - The error constructors used to thow (e.g. `new WebAssembly.CompileError()`).

thow => throw

> Source/JavaScriptCore/ChangeLog:9
> +         - The error constructors used to thow (e.g. `new WebAssembly.CompileError()`).

Ditto.
Comment 5 JF Bastien 2016-10-20 17:42:40 PDT 
Created attachment 292293 [details]
patch

Fix typo.
Comment 6 WebKit Commit Bot 2016-10-20 17:43:42 PDT 
Comment on attachment 292293 [details]
patch

Rejecting attachment 292293 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 292293, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in JSTests/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/2334501
Comment 7 JF Bastien 2016-10-20 17:45:55 PDT 
I filed the following to fix .name separately: https://bugs.webkit.org/show_bug.cgi?id=163768
Comment 8 JF Bastien 2016-10-20 17:47:30 PDT 
Created attachment 292295 [details]
patch

Fix reviewer. I'll get used to this workflow some day...
Comment 9 WebKit Commit Bot 2016-10-20 18:22:09 PDT 
Comment on attachment 292295 [details]
patch

Clearing flags on attachment: 292295

Committed r207650: <http://trac.webkit.org/changeset/207650>
Comment 10 WebKit Commit Bot 2016-10-20 18:22:14 PDT 
All reviewed patches have been landed.  Closing bug.