[webkit-reviews] review granted: [Bug 74341] Start de-virtualizing destructors starting with JSCell and moving down : [Attachment 118864] Start de-virtualizing destructors

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 74341: Start de-virtualizing destructors starting with JSCell and moving
down
https://bugs.webkit.org/show_bug.cgi?id=74341

Attachment 118864: Start de-virtualizing destructors
https://bugs.webkit.org/attachment.cgi?id=118864&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=118864&action=review


Remember to remove the #pragmas in your final patch.

> Source/JavaScriptCore/heap/MarkedBlock.cpp:72
> +    if (cell->classInfo() != &JSFinalObject::s_info)

Use isJSFinalObject().

> Source/JavaScriptCore/runtime/Executable.cpp:47
> +    jsCast<ExecutableBase*>(cell)->~ExecutableBase();

Should be ExecutableBase::~ExecutableBase().

> Source/JavaScriptCore/runtime/JSArray.cpp:280
> +    ASSERT(classInfo()->isSubClassOf(&JSArray::s_info));

I think this would read better as ASSERT(jsCast<JSArray>(this)).

> Source/JavaScriptCore/runtime/JSByteArray.cpp:47
> +    ASSERT(classInfo()->isSubClassOf(&JSByteArray::s_info));

Ditto.

> Source/JavaScriptCore/runtime/JSObject.cpp:90
> +    jsCast<JSObject*>(cell)->~JSObject();

JSObject::~JSObject().

> Source/JavaScriptCore/runtime/JSString.cpp:55
> +    ASSERT(thisObject->classInfo() == &JSString::s_info);

I think this ASSERT would be more readable as ASSERT(jsCast<JSString>(cell)).