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
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
Patch (70.30 KB, patch)
2019-11-15 13:51 PST, Ross Kirsling
no flags
Patch (70.33 KB, patch)
2019-11-15 13:57 PST, Ross Kirsling
no flags
Patch (70.31 KB, patch)
2019-11-15 14:01 PST, Ross Kirsling
no flags
Patch (77.73 KB, patch)
2019-11-15 16:33 PST, Ross Kirsling
no flags
Patch for landing (78.25 KB, patch)
2019-11-15 18:58 PST, Ross Kirsling
no flags
Ross Kirsling
Comment 1 2019-11-14 17:29:09 PST
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)
EWS Watchlist
Comment 4 2019-11-14 22:36:16 PST Comment hidden (obsolete)
Ross Kirsling
Comment 5 2019-11-15 13:51:33 PST
Ross Kirsling
Comment 6 2019-11-15 13:57:12 PST
Ross Kirsling
Comment 7 2019-11-15 14:01:19 PST
Ross Kirsling
Comment 8 2019-11-15 16:33:01 PST
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
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
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.