Bug 165219 - WebAssembly builder: don't throw when checker not implemented
Summary: WebAssembly builder: don't throw when checker not implemented
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: 163421
  Show dependency treegraph
 
Reported: 2016-11-30 13:54 PST by JF Bastien
Modified: 2016-11-30 17:03 PST (History)
3 users (show)

See Also:


Attachments
patch (24.73 KB, patch)
2016-11-30 13:56 PST, JF Bastien
mark.lam: review+
Details | Formatted Diff | Diff
patch (24.79 KB, patch)
2016-11-30 16:26 PST, JF Bastien
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description JF Bastien 2016-11-30 13:54:41 PST
We should perform whichever checks we've implemented, and assume the rest are OK until bug #163421 is fixed.
Comment 1 JF Bastien 2016-11-30 13:56:43 PST
Created attachment 295762 [details]
patch
Comment 2 Mark Lam 2016-11-30 15:22:57 PST
Comment on attachment 295762 [details]
patch

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

r=me with fixes.

> JSTests/ChangeLog:6
> +        Reviewed by Keith Miller.

Did Keith already review this?

> JSTests/wasm/function-tests/br-if-loop-less-than.js:51
> -                                      ]);
>  \ No newline at end of file
> +                                      ]);

What's with the "\ No newline at end of file"?  Can you add a newline at the end and remove this?  Same for the other instances of this below.

> JSTests/wasm/self-test/test_BuilderJSON.js:414
> +    assertOpThrows(f => f.I32Const(), `Not the same: "0" and "1": "i32.const" expects exactly 1 immediates, got 0`);

/immediates/immediate/?
Comment 3 JF Bastien 2016-11-30 16:26:41 PST
Created attachment 295784 [details]
patch

(In reply to comment #2)
> Comment on attachment 295762 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=295762&action=review
> 
> r=me with fixes.

ty!


> > JSTests/ChangeLog:6
> > +        Reviewed by Keith Miller.
> 
> Did Keith already review this?

I was optimistic, changed to you :-)


> > JSTests/wasm/function-tests/br-if-loop-less-than.js:51
> > -                                      ]);
> >  \ No newline at end of file
> > +                                      ]);
> 
> What's with the "\ No newline at end of file"?  Can you add a newline at the
> end and remove this?  Same for the other instances of this below.

Super weird. I think the original file didn't have it, and now the diff tool is trying to be helpful because my editor adds the newlines automatically, and it's just being confusing.


> > JSTests/wasm/self-test/test_BuilderJSON.js:414
> > +    assertOpThrows(f => f.I32Const(), `Not the same: "0" and "1": "i32.const" expects exactly 1 immediates, got 0`);
> 
> /immediates/immediate/?

Done (I pluralized all non-1 things including 0).
Comment 4 WebKit Commit Bot 2016-11-30 17:03:22 PST
Comment on attachment 295784 [details]
patch

Clearing flags on attachment: 295784

Committed r209165: <http://trac.webkit.org/changeset/209165>
Comment 5 WebKit Commit Bot 2016-11-30 17:03:26 PST
All reviewed patches have been landed.  Closing bug.