RESOLVED FIXED 165219
WebAssembly builder: don't throw when checker not implemented 
https://bugs.webkit.org/show_bug.cgi?id=165219
Summary WebAssembly builder: don't throw when checker not implemented
JF Bastien
Reported 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.
Attachments
patch (24.73 KB, patch)
2016-11-30 13:56 PST, JF Bastien 		mark.lam: review+
DetailsFormatted DiffDiff
patch (24.79 KB, patch)
2016-11-30 16:26 PST, JF Bastien 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
JF Bastien
Comment 1 2016-11-30 13:56:43 PST
Created attachment 295762 [details] patch
Mark Lam
Comment 2 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/?
JF Bastien
Comment 3 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).
WebKit Commit Bot
Comment 4 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>
WebKit Commit Bot
Comment 5 2016-11-30 17:03:26 PST
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.