WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
223626
[JSC] Use ErrorInstance for AggregateError
https://bugs.webkit.org/show_bug.cgi?id=223626
Summary
[JSC] Use ErrorInstance for AggregateError
Yusuke Suzuki
Reported
2021-03-22 23:28:30 PDT
[JSC] Use ErrorInstance for AggregateError
Attachments
Patch
(8.68 KB, patch)
2021-03-22 23:31 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1
2021-03-22 23:31:21 PDT
Created
attachment 423990
[details]
Patch
Darin Adler
Comment 2
2021-03-23 10:03:03 PDT
Comment on
attachment 423990
[details]
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.
Yusuke Suzuki
Comment 3
2021-03-23 12:49:31 PDT
Comment on
attachment 423990
[details]
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.
This static_cast is necessary since there is `JS_EXPORT_PRIVATE JSArray* constructArray(JSGlobalObject*, Structure*, const ArgList& values);`.
>> 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.
We should consider this option at some point.
EWS
Comment 4
2021-03-23 12:57:58 PDT
Committed
r274893
: <
https://commits.webkit.org/r274893
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 423990
[details]
.
Radar WebKit Bug Importer
Comment 5
2021-03-23 12:58:41 PDT
<
rdar://problem/75750983
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug