[webkit-reviews] review granted: [Bug 173287] WebAssembly: import updated spec tests : [Attachment 312735] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jun 12 17:40:02 PDT 2017
Saam Barati <sbarati at apple.com> has granted JF Bastien <jfbastien at apple.com>'s
request for review:
Bug 173287: WebAssembly: import updated spec tests
https://bugs.webkit.org/show_bug.cgi?id=173287
Attachment 312735: patch
https://bugs.webkit.org/attachment.cgi?id=312735&action=review
--- Comment #10 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 312735
--> https://bugs.webkit.org/attachment.cgi?id=312735
patch
View in context: https://bugs.webkit.org/attachment.cgi?id=312735&action=review
r=me
> Source/JavaScriptCore/wasm/js/JSWebAssemblyTable.cpp:102
> + if (std::numeric_limits<uint32_t>::max() - m_size < delta)
> + return false;
Please used Checked instead. I think it's easier to read, and it's our way of
detecting overflow.
> Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:258
> + ?
static_cast<uint32_t>(m_instance->loadI32Global(segment->offset.globalImportInd
ex()))
> + : segment->offset.constValue();
indentation is wrong
> Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:280
> + if (UNLIKELY(sizeInBytes < segment->sizeInBytes))
> + exception = dataSegmentFail(exec, vm, scope, sizeInBytes,
segment->sizeInBytes, offset, ASCIILiteral(", segment is too big"));
you should return early here as to not throw another exception over the current
one.
> Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:94
> + if (UNLIKELY(scope.exception())) {
> + reject(exec, scope, promise);
> + } else {
no braces here.
> Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:118
> + RETURN_IF_EXCEPTION(scope, reject(exec, scope, promise));
Ignore my previous comment. This code is more terse. I like it.
> Source/JavaScriptCore/wasm/js/WebAssemblyPrototype.cpp:185
> + RETURN_IF_EXCEPTION(scope, encodedJSValue());
Since you're touching this code, might as well change it to use "{ }" instead
of "encodedJSValue()"
More information about the webkit-reviews
mailing list