[webkit-reviews] review granted: [Bug 170210] LinkBuffer and ExecutableAllocator shouldn't have anything to do with VM : [Attachment 305788] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Mar 29 14:28:13 PDT 2017


Mark Lam <mark.lam at apple.com> has granted Saam Barati <sbarati at apple.com>'s
request for review:
Bug 170210: LinkBuffer and ExecutableAllocator shouldn't have anything to do
with VM
https://bugs.webkit.org/show_bug.cgi?id=170210

Attachment 305788: patch

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




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

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

r=me with suggestions.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:5984
> +	   VM* vm = &this->vm();

nit: can we use a VM& instead?

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6282
> +	   VM* vm = &this->vm();

Ditto.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6408
> +	   VM* vm = &this->vm();

Ditto.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:6678
> +	   VM* vm = &this->vm();

Ditto.

> Source/JavaScriptCore/jit/ExecutableAllocator.h:141
> +private:
> +

nit: can you put the empty line before the private: section instead of after?

> Source/JavaScriptCore/runtime/VM.cpp:169
>  #if ENABLE(ASSEMBLER)
> -    , executableAllocator(*this)
>  #endif

Remove these lines altogether.


More information about the webkit-reviews mailing list