[webkit-reviews] review granted: [Bug 122566] FTL: Soft-link LLVM as a workaround for LLVM's static initializers and exit-time destructors : [Attachment 213834] the patch

[bugzilla-daemon at webkit.org]

Oliver Hunt <oliver at apple.com> has granted Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 122566: FTL: Soft-link LLVM as a workaround for LLVM's static initializers
and exit-time destructors
https://bugs.webkit.org/show_bug.cgi?id=122566

Attachment 213834: the patch
https://bugs.webkit.org/attachment.cgi?id=213834&action=review

------- Additional Comments from Oliver Hunt <oliver at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=213834&action=review


> Source/JavaScriptCore/jsc.cpp:849
> -    RefPtr<VM> vm = VM::create(LargeHeap);
> +    VM* vm = VM::create(LargeHeap).leakRef();

Do we really have to leak the vm?

>> Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:125
>> +	macro( LLVMValueRef , IsABasicBlock, (LLVMValueRef Val)) \
> 
> Extra space after ( in function call	[whitespace/parens] [4]

Please remove these extra spaces, also change " , " to ", "

> Source/JavaScriptCore/llvm/library/LLVMOverrides.cpp:33
> +extern "C" int __cxa_atexit() { return 0; }

mmmm, tasty