Bug 170210 – LinkBuffer and ExecutableAllocator shouldn't have anything to do with VM

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Saam Barati]

...

[Saam Barati]

Created attachment 305695 [details]
WIP

[Saam Barati]

Created attachment 305779 [details]
WIP

lets see what EWS thinks

[Saam Barati]

Created attachment 305780 [details]
WIP

added webcore change

[Saam Barati]

Created attachment 305783 [details]
WIP

fixed some bad lambda capture

[Saam Barati]

Created attachment 305788 [details]
patch

[Build Bot]

Attachment 305788 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 1 in 52 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Lam]

Comment on attachment 305788 [details]
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.

[Saam Barati]

Comment on attachment 305788 [details]
patch

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

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:5984
>> +        VM* vm = &this->vm();
> 
> nit: can we use a VM& instead?

The reason I did this is that changing to VM& would require 2 more lines of code. And it's also a bit more in line with the style in FTLLower to get a pointer. In principle, I agree, though. We know the thing will never be null.

[Saam Barati]

Created attachment 305794 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 305794 [details]
patch for landing

Clearing flags on attachment: 305794

Committed r214571: <http://trac.webkit.org/changeset/214571>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.