WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
216637
[JSC] Optimize Promise#finally by avoiding creating multiple environments
https://bugs.webkit.org/show_bug.cgi?id=216637
Summary
[JSC] Optimize Promise#finally by avoiding creating multiple environments
Yusuke Suzuki
Reported
2020-09-16 23:00:26 PDT
[JSC] Optimize Promise#finally by avoiding creating multiple environments
Attachments
Patch
(3.01 KB, patch)
2020-09-16 23:07 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1
2020-09-16 23:07:21 PDT
Created
attachment 408992
[details]
Patch
Ross Kirsling
Comment 2
2020-09-16 23:35:56 PDT
Comment on
attachment 408992
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=408992&action=review
Seems fine to me.
> Source/JavaScriptCore/builtins/PromisePrototype.js:74 > + thenFinally = (0, /* prevent function name inference */ (value) => {
nit: Seems like this could be a line comment above? But I don't feel that strongly.
Yusuke Suzuki
Comment 3
2020-09-16 23:54:29 PDT
Comment on
attachment 408992
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=408992&action=review
>> Source/JavaScriptCore/builtins/PromisePrototype.js:74 >> + thenFinally = (0, /* prevent function name inference */ (value) => { > > nit: Seems like this could be a line comment above? But I don't feel that strongly.
I think this is because `(0, expr)` looks super tricky, and this inlined comment helps readers to understand why this comma expression exists. I think this prefixed comment is nice for consistency since we are using this style in the other places.
EWS
Comment 4
2020-09-17 01:06:05 PDT
Committed
r267184
: <
https://trac.webkit.org/changeset/267184
> All reviewed patches have been landed. Closing bug and clearing flags on
attachment 408992
[details]
.
Radar WebKit Bug Importer
Comment 5
2020-09-17 01:07:21 PDT
<
rdar://problem/69049394
>
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