[webkit-reviews] review granted: [Bug 233918] [JSC] Introduce WriteBarrierStructureID : [Attachment 446273] Patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 233918: [JSC] Introduce WriteBarrierStructureID
https://bugs.webkit.org/show_bug.cgi?id=233918

Attachment 446273: Patch

https://bugs.webkit.org/attachment.cgi?id=446273&action=review




--- Comment #6 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 446273
  --> https://bugs.webkit.org/attachment.cgi?id=446273
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=446273&action=review

r=me

> Source/JavaScriptCore/runtime/WriteBarrier.h:278
> +    // Should only be used by JSCell during early initialisation
> +    // when some basic types aren't yet completely instantiated

Why this comment?  In this patch, `setEarlyValue` looks more like a
`setInternal` and can be made private.	Is this needed in code that is yet to
come?

> Source/JavaScriptCore/runtime/WriteBarrier.h:283
> +	   // Copy m_cell to a local to avoid multiple-read issues. (See
<http://webkit.org/b/110854>)

/m_cell/m_structureID/

> Source/JavaScriptCore/runtime/WriteBarrier.h:313
> +	   m_structureID = StructureID { };

Do we need the `StructureID` here?  Wouldn't it be implied by the type of
m_structureID?

> Source/JavaScriptCore/runtime/WriteBarrier.h:332
> +	       m_structureID = StructureID { };

Ditto.

> Source/JavaScriptCore/runtime/WriteBarrierInlines.h:82
> +	   m_structureID = StructureID { };

Ditto.