[webkit-reviews] review denied: [Bug 65288] Add checks to ensure allocation does not take place during initialization of GC-managed objects : [Attachment 102977] Fixing windows again

[bugzilla-daemon at webkit.org]

Oliver Hunt <oliver at apple.com> has denied Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 65288: Add checks to ensure allocation does not take place during
initialization of GC-managed objects
https://bugs.webkit.org/show_bug.cgi?id=65288

Attachment 102977: Fixing windows again
https://bugs.webkit.org/attachment.cgi?id=102977&action=review

------- Additional Comments from Oliver Hunt <oliver at apple.com>
One thing i have a problem with is the inconsistency of initProperties() vs.
inlining initialisation into the create() method.  The inlining of anything
into a create method potentially breaks subclassing.

I think a much better approach would be to
* Give JSCell an initProperties() method
* require every initProperties method to do BaseObject::initProperties(...)
first
* Then every create() method can follow the pattern
    JSFoo* foo = new (allocateCell<JSFoo>(*exec->heap())) JSFoo(...);
    foo->initProperties(...);
    return foo;
  Then we can also add an assertion to the base JSCell::initProperties to
ensure we have actually completed initialisation if we wanted to.  But even if
we don't do that, we can look at create methods, and easily see obvious errors.