Bug 163699

Summary:

WebAssembly API: implement exception constructors properly

Product:

WebKit

Reporter:

JF Bastien <jfbastien>

Component:

JavaScriptCore

Assignee:

JF Bastien <jfbastien>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, fpizlo, ggaren, keith_miller, mark.lam, msaboff, saam, ysuzuki

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

161709, 163768

Attachments:

Description	Flags
patch	none
patch	keith_miller: review+, keith_miller: commit-queue-
patch	commit-queue: commit-queue-
patch	none

JF Bastien

Reported 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

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 attachment proposed patch, testcase, etc.

JF Bastien

Comment 1 2016-10-20 16:55:09 PDT

Created attachment 292286 [details] patch

JF Bastien

Comment 2 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?

JF Bastien

Comment 3 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.

Keith Miller

Comment 4 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.

JF Bastien

Comment 5 2016-10-20 17:42:40 PDT

Created attachment 292293 [details] patch Fix typo.

WebKit Commit Bot

Comment 6 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

JF Bastien

Comment 7 2016-10-20 17:45:55 PDT

I filed the following to fix .name separately: https://bugs.webkit.org/show_bug.cgi?id=163768

JF Bastien

Comment 8 2016-10-20 17:47:30 PDT

Created attachment 292295 [details] patch Fix reviewer. I'll get used to this workflow some day...

WebKit Commit Bot

Comment 9 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>

WebKit Commit Bot

Comment 10 2016-10-20 18:22:14 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.