In order to preserve the correctness across patches, we'll start de-virtualizing destructors starting with JSCell and moving down the hierarchy using the destroy functions. Any class in the next level down that we haven't de-virtualized yet will receive a static destroy function that calls the virtual destructor for that subtree of the hierarchy. Any class in the current level or above that has a non-empty destructor (i.e. it has members that require calls to their destructors or any other pre-existing logic) will receive a destroy method that statically calls that class's destructor, whether that destructor is implicitly or explicitly defined. Any class with what C++ calls a non-trivial destructor will receive a COMPILE_ASSERT stating as much. In a future patch sequence, we will move toward making all classes in the JSCell hierarchy have non-trivial destructors. We must keep a virtual vtableAnchor() function rooted in JSCell to prevent objects from losing the vptrs, since we're not quite ready for them to disappear yet. We will use #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wnon-virtual-dtor" // class declaration... #pragma clang diagnostic pop to avoid errors due to the fact that classes will still have a virtual method but no virtual destructor.
Created attachment 118864 [details] Start de-virtualizing destructors No changelog (see bug 74331)
Comment on attachment 118864 [details] Start de-virtualizing destructors 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)).