This information is necessary for the GC work that Oliver is doing. We'll add the relevant information to the ClassInfo struct.
Created attachment 109828 [details] Patch
Comment on attachment 109828 [details] Patch I think a simpler approach would just be to make the method table contain this information, as CREATE_METHOD_TABLE already has the class name
> I think a simpler approach would just be to make the method table contain this information, as CREATE_METHOD_TABLE already has the class name :-(
Created attachment 109834 [details] Patch
Comment on attachment 109834 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109834&action=review > Source/JavaScriptCore/runtime/WriteBarrier.h:50 > + ASSERT(WTF::RemovePointer<T>::Type::s_info.cellSize == sizeof(typename WTF::RemovePointer<T>::Type)); This isn't a safe assertion class A { ... } class B : A { int aNewField; } validateCell<A*>(&someB) would fail this assertion as B's classinfo will (correctly) report a larger cell size than A's.
Comment on attachment 109834 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109834&action=review > Source/JavaScriptCore/runtime/JSCell.h:66 > + size_t cellSize() const; I don't think JSCell needs this helper function. We can write classInfo()->cellSize at the call site. >> Source/JavaScriptCore/runtime/WriteBarrier.h:50 >> + ASSERT(WTF::RemovePointer<T>::Type::s_info.cellSize == sizeof(typename WTF::RemovePointer<T>::Type)); > > This isn't a safe assertion > > class A { > ... > } > > class B : A { > int aNewField; > } > > validateCell<A*>(&someB) > > would fail this assertion as B's classinfo will (correctly) report a larger cell size than A's. Maybe you can turn this ASSERT into >=? That would still catch some bugs. A better place for an ASSERT might be allocateCell<T>: ASSERT(T::s_info.cellSize == sizeof(T)).
(In reply to comment #6) > (From update of attachment 109834 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=109834&action=review > > > Source/JavaScriptCore/runtime/JSCell.h:66 > > + size_t cellSize() const; > > I don't think JSCell needs this helper function. We can write classInfo()->cellSize at the call site. > > >> Source/JavaScriptCore/runtime/WriteBarrier.h:50 > >> + ASSERT(WTF::RemovePointer<T>::Type::s_info.cellSize == sizeof(typename WTF::RemovePointer<T>::Type)); > > > > This isn't a safe assertion > > > > class A { > > ... > > } > > > > class B : A { > > int aNewField; > > } > > > > validateCell<A*>(&someB) > > > > would fail this assertion as B's classinfo will (correctly) report a larger cell size than A's. > > Maybe you can turn this ASSERT into >=? That would still catch some bugs. > > A better place for an ASSERT might be allocateCell<T>: ASSERT(T::s_info.cellSize == sizeof(T)). Yeah i spoke to mark and that's what he's doing :D
Created attachment 109855 [details] Patch
Comment on attachment 109855 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109855&action=review > Source/JavaScriptCore/runtime/JSObject.h:494 > +inline size_t JSCell::cellSize() const > +{ > + return classInfo()->cellSize; > +} This function is unused. I still think it can be inlined at the call site when used.
Created attachment 109860 [details] Patch
Comment on attachment 109860 [details] Patch r=me
Created attachment 109904 [details] Fixing windows
Comment on attachment 109904 [details] Fixing windows r=me
Comment on attachment 109904 [details] Fixing windows Clearing flags on attachment: 109904 Committed r96831: <http://trac.webkit.org/changeset/96831>
All reviewed patches have been landed. Closing bug.