Bug 224247

Summary: Remove className() and toStringName() from the method table
Product: WebKit Reporter: Alexey Shvayka <ashvayka>
Component: JavaScriptCoreAssignee: Alexey Shvayka <ashvayka>
Status: RESOLVED FIXED    
Severity: Enhancement CC: darin, ews-watchlist, hi, joepeck, keith_miller, mark.lam, msaboff, saam, smoley, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=199138
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

Description Alexey Shvayka 2021-04-06 13:59:17 PDT 
Remove className() and toStringName() from the method table
Comment 1 Alexey Shvayka 2021-04-06 14:01:56 PDT 
Created attachment 425324 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2021-04-07 14:55:03 PDT 
<rdar://problem/76366903>
Comment 3 Yusuke Suzuki 2021-04-07 15:06:18 PDT 
That's awesome.
Comment 4 Alexey Shvayka 2021-04-08 15:24:41 PDT 
Created attachment 425548 [details]
Patch

Fix testapi and iOS build, add missing exception check and tests.
Comment 5 Darin Adler 2021-04-08 15:43:55 PDT 
Comment on attachment 425548 [details]
Patch

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

> Source/JavaScriptCore/runtime/ObjectPrototype.cpp:329
> +inline const char* getBuiltinTag(JSGlobalObject* globalObject, JSObject* object)

WebKit coding style discourages use of the word "get" in the name of a function like this.

> Source/JavaScriptCore/runtime/ObjectPrototype.cpp:390
> +        jsTag = jsNontrivialString(vm, String(tag));

I am slightly surprised that an explicit cast to String is needed here. Can we try this without the function-style cast to String?

Seems slightly unfortunate that we always have to pay the price of allocating a StringImpl every time this code is called, given that there is a small fixed set of strings this can ever return. Probably OK that we copy the characters into the StringImpl and don’t do the ASCIILiteral optimization.
Comment 6 Alexey Shvayka 2021-04-08 17:16:48 PDT 
Created attachment 425564 [details]
Patch for landing
Comment 7 Alexey Shvayka 2021-04-08 17:17:28 PDT 
(In reply to Darin Adler from comment #5)

Thank you for review, Darin!

> WebKit coding style discourages use of the word "get" in the name of a
> function like this.

Renamed to "inferBuiltinTag".

> I am slightly surprised that an explicit cast to String is needed here. Can
> we try this without the function-style cast to String?

Implicit cast works.

> Seems slightly unfortunate that we always have to pay the price of
> allocating a StringImpl every time this code is called, given that there is
> a small fixed set of strings this can ever return.

Great suggestion! Changed to AtomStringImpl so StringImpl allocation can be avoided.
Also, removed jsNontrivialString() because its assumption of the string length can be broken in case of iOS hack + API object.
Comment 8 EWS 2021-04-09 18:00:23 PDT 
Committed r275788 (236359@main): <https://commits.webkit.org/236359@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 425564 [details].