WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 204214
[JSC] Anonymous built-in functions should have empty string for a name
https://bugs.webkit.org/show_bug.cgi?id=204214
Summary
[JSC] Anonymous built-in functions should have empty string for a name
Ross Kirsling
Reported
2019-11-14 17:07:26 PST
[JSC] Anonymous built-in functions should have empty string for a name
Attachments
Patch
(70.86 KB, patch)
2019-11-14 17:29 PST
,
Ross Kirsling
no flags
Details
Formatted Diff
Diff
Archive of layout-test-results from ews213 for win-future
(14.21 MB, application/zip)
2019-11-14 22:36 PST
,
EWS Watchlist
no flags
Details
Patch
(70.30 KB, patch)
2019-11-15 13:51 PST
,
Ross Kirsling
no flags
Details
Formatted Diff
Diff
Patch
(70.33 KB, patch)
2019-11-15 13:57 PST
,
Ross Kirsling
no flags
Details
Formatted Diff
Diff
Patch
(70.31 KB, patch)
2019-11-15 14:01 PST
,
Ross Kirsling
no flags
Details
Formatted Diff
Diff
Patch
(77.73 KB, patch)
2019-11-15 16:33 PST
,
Ross Kirsling
no flags
Details
Formatted Diff
Diff
Patch for landing
(78.25 KB, patch)
2019-11-15 18:58 PST
,
Ross Kirsling
no flags
Details
Formatted Diff
Diff
Show Obsolete
(6)
View All
Add attachment
proposed patch, testcase, etc.
Ross Kirsling
Comment 1
2019-11-14 17:29:09 PST
Created
attachment 383590
[details]
Patch
Ross Kirsling
Comment 2
2019-11-14 22:02:00 PST
Note: These fixes are associated with
https://github.com/tc39/ecma262/pull/1490
.
EWS Watchlist
Comment 3
2019-11-14 22:36:13 PST
Comment hidden (obsolete)
Comment on
attachment 383590
[details]
Patch
Attachment 383590
[details]
did not pass win-ews (win): Output:
https://webkit-queues.webkit.org/results/13257634
New failing tests: imported/blink/fast/events/panScroll-crash.html
EWS Watchlist
Comment 4
2019-11-14 22:36:16 PST
Comment hidden (obsolete)
Created
attachment 383601
[details]
Archive of layout-test-results from ews213 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews213 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Ross Kirsling
Comment 5
2019-11-15 13:51:33 PST
Created
attachment 383643
[details]
Patch
Ross Kirsling
Comment 6
2019-11-15 13:57:12 PST
Created
attachment 383648
[details]
Patch
Ross Kirsling
Comment 7
2019-11-15 14:01:19 PST
Created
attachment 383649
[details]
Patch
Ross Kirsling
Comment 8
2019-11-15 16:33:01 PST
Created
attachment 383663
[details]
Patch
Yusuke Suzuki
Comment 9
2019-11-15 18:06:12 PST
Comment on
attachment 383663
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=383663&action=review
r=me with nits.
> Source/JavaScriptCore/builtins/PromiseConstructor.js:244 > - function @resolve(resolution) { > + function (resolution) { > return @resolvePromiseWithFirstResolvingFunctionCallCheck(capturedPromise, resolution); > }, > - function @reject(reason) { > + function (reason) { > return @rejectPromiseWithFirstResolvingFunctionCallCheck(capturedPromise, reason);
You need to change InternalPromise constructor's one too.
> Source/JavaScriptCore/runtime/ProxyRevoke.cpp:54 > + Base::finishCreation(vm, ""_s);
Pass `emptyString()`.
Ross Kirsling
Comment 10
2019-11-15 18:58:24 PST
Created
attachment 383678
[details]
Patch for landing
WebKit Commit Bot
Comment 11
2019-11-15 19:44:34 PST
Comment on
attachment 383678
[details]
Patch for landing Clearing flags on attachment: 383678 Committed
r252520
: <
https://trac.webkit.org/changeset/252520
>
WebKit Commit Bot
Comment 12
2019-11-15 19:44:36 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 13
2019-11-15 19:45:24 PST
<
rdar://problem/57248654
>
Darin Adler
Comment 14
2019-11-18 09:23:06 PST
Comment on
attachment 383678
[details]
Patch for landing View in context:
https://bugs.webkit.org/attachment.cgi?id=383678&action=review
> Source/JavaScriptCore/runtime/FunctionRareData.h:129 > + FunctionRareData(VM&);
In a case like this it’s good to add "explicit" since this is now a single-argument constructor and "explicit" has value to make sure those aren’t accidentally used for type conversion. On the other hand, with modern C++ we could be using explicit a lot more on most constructors that aren’t intended to be used for type conversion, even ones that don’t have a single argument, and that would affect a lot of code and not sure everyone agrees this would be good.
Darin Adler
Comment 15
2019-11-18 09:24:06 PST
Comment on
attachment 383678
[details]
Patch for landing View in context:
https://bugs.webkit.org/attachment.cgi?id=383678&action=review
> Source/JavaScriptCore/runtime/JSFunctionInlines.h:109 > + return m_rareData ? m_rareData->hasReifiedName() : false;
I personally like the && syntax for cases like this better than ? : like this and find that it is easy to understand and reason about: return m_rareData && m_rareData->hasReifiedName();
Ross Kirsling
Comment 16
2019-11-18 09:49:59 PST
(In reply to Darin Adler from
comment #14
)
> Comment on
attachment 383678
[details]
> Patch for landing > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=383678&action=review
> > > Source/JavaScriptCore/runtime/FunctionRareData.h:129 > > + FunctionRareData(VM&); > > In a case like this it’s good to add "explicit" since this is now a > single-argument constructor and "explicit" has value to make sure those > aren’t accidentally used for type conversion.
Good point! Will address. (In reply to Darin Adler from
comment #15
)
> Comment on
attachment 383678
[details]
> Patch for landing > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=383678&action=review
> > > Source/JavaScriptCore/runtime/JSFunctionInlines.h:109 > > + return m_rareData ? m_rareData->hasReifiedName() : false; > > I personally like the && syntax for cases like this better than ? : like > this and find that it is easy to understand and reason about: > > return m_rareData && m_rareData->hasReifiedName();
I guess I was thinking in JavaScript mode where this wouldn't return the right type, but I guess the coercion works to one's advantage in C++, doesn't it. :P
Ross Kirsling
Comment 17
2019-11-18 10:01:25 PST
Committed
r252547
: <
https://trac.webkit.org/changeset/252547
>
Darin Adler
Comment 18
2019-11-18 17:04:42 PST
(In reply to Ross Kirsling from
comment #16
)
> (In reply to Darin Adler from
comment #14
) > > I personally like the && syntax for cases like this better than ? : like > > this and find that it is easy to understand and reason about: > > > > return m_rareData && m_rareData->hasReifiedName(); > > I guess I was thinking in JavaScript mode where this wouldn't return the > right type, but I guess the coercion works to one's advantage in C++, > doesn't it. :P
If the "coercion" wasn't present in C++ I would probably write: return m_rareData != nullptr && m_rareData->hasReifiedName();
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