Bug 224247 – Remove className() and toStringName() from the method table

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Alexey Shvayka]

Remove className() and toStringName() from the method table

[Alexey Shvayka]

Created attachment 425324 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/76366903>

[Yusuke Suzuki]

That's awesome.

[Alexey Shvayka]

Created attachment 425548 [details]
Patch

Fix testapi and iOS build, add missing exception check and tests.

[Darin Adler]

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.

[Alexey Shvayka]

Created attachment 425564 [details]
Patch for landing

[Alexey Shvayka]

(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.

[EWS]

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].