[webkit-reviews] review granted: [Bug 223626] [JSC] Use ErrorInstance for AggregateError : [Attachment 423990] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 223626: [JSC] Use ErrorInstance for AggregateError
https://bugs.webkit.org/show_bug.cgi?id=223626

Attachment 423990: Patch

https://bugs.webkit.org/attachment.cgi?id=423990&action=review




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 423990
  --> https://bugs.webkit.org/attachment.cgi?id=423990
Patch

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

> Source/JavaScriptCore/runtime/AggregateError.cpp:54
> +    auto* array = constructArray(globalObject,
static_cast<ArrayAllocationProfile*>(nullptr), errorsList);

Surprised that we need the ArrayAllocationProfile cast. I don’t see so much
overloading for constructArray that I would have expected ambiguity.

> Source/JavaScriptCore/runtime/AggregateError.cpp:58
> +    error->putDirect(vm, vm.propertyNames->errors, array,
static_cast<unsigned>(PropertyAttribute::DontEnum));

We should consider moving from unsigned to OptionSet for property attributes.
Not sure how people who regularly work on JavaScriptCore feel about that, but I
would love it.