[webkit-reviews] review granted: [Bug 207087] [JSC] Introduce UnlinkedCodeBlockGenerator and reduce sizeof(UnlinkedCodeBlock) : [Attachment 389645] Patch

[bugzilla-daemon at webkit.org]

Tadeu Zagallo <tzagallo at apple.com> has granted Yusuke Suzuki
<ysuzuki at apple.com>'s request for review:
Bug 207087: [JSC] Introduce UnlinkedCodeBlockGenerator and reduce
sizeof(UnlinkedCodeBlock)
https://bugs.webkit.org/show_bug.cgi?id=207087

Attachment 389645: Patch

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




--- Comment #7 from Tadeu Zagallo <tzagallo at apple.com> ---
Comment on attachment 389645
  --> https://bugs.webkit.org/attachment.cgi?id=389645
Patch

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

Nice!

> Source/JavaScriptCore/bytecode/UnlinkedCodeBlockGenerator.cpp:150
> +	   if (!m_codeBlock->m_rareData) {

Why would the code block have rare data at this point?

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:291
> +    m_codeBlock->finalize(m_writer.finalize());

Not necessarily on this patch, but ideally I think UnlinkedCodeBlockGenerator
should own the UnlinkedCodeBlock and return it from finalize.

> Source/WTF/wtf/RefCountedArray.h:201
> +    T& front() { return (*this)[0]; }
> +    const T& front() const { return (*this)[0]; }

I believe this called `first` in Vector, should we just call the same here?