Bug 122566

Created attachment 213799 [details] work in progress

Created attachment 213829 [details] almost done

Created attachment 213834 [details] the patch

Attachment 213834 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig', u'Source/JavaScriptCore/Configurations/LLVMForJSC.xcconfig', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/config.h', u'Source/JavaScriptCore/dfg/DFGPlan.cpp', u'Source/JavaScriptCore/disassembler/LLVMDisassembler.cpp', u'Source/JavaScriptCore/ftl/FTLAbbreviatedTypes.h', u'Source/JavaScriptCore/ftl/FTLAbbreviations.h', u'Source/JavaScriptCore/ftl/FTLCompile.cpp', u'Source/JavaScriptCore/ftl/FTLFail.cpp', u'Source/JavaScriptCore/ftl/FTLJITCode.h', u'Source/JavaScriptCore/ftl/FTLJITFinalizer.h', u'Source/JavaScriptCore/ftl/FTLLink.cpp', u'Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.h', u'Source/JavaScriptCore/ftl/FTLState.cpp', u'Source/JavaScriptCore/ftl/WebKitLLVMLibraryAnchor.cpp', u'Source/JavaScriptCore/jsc.cpp', u'Source/JavaScriptCore/llvm/InitializeLLVM.cpp', u'Source/JavaScriptCore/llvm/InitializeLLVM.h', u'Source/JavaScriptCore/llvm/InitializeLLVMMac.mm', u'Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp', u'Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.h', u'Source/JavaScriptCore/llvm/LLVMAPI.cpp', u'Source/JavaScriptCore/llvm/LLVMAPI.h', u'Source/JavaScriptCore/llvm/LLVMAPIFunctions.h', u'Source/JavaScriptCore/llvm/LLVMHeaders.h', u'Source/JavaScriptCore/llvm/library/LLVMAnchor.cpp', u'Source/JavaScriptCore/llvm/library/LLVMExports.cpp', u'Source/JavaScriptCore/llvm/library/LLVMOverrides.cpp', u'Source/JavaScriptCore/runtime/InitializeThreading.cpp', u'Source/JavaScriptCore/runtime/Options.h', u'Source/WTF/ChangeLog', u'Source/WTF/WTF.xcodeproj/project.pbxproj', u'Source/WTF/wtf/LLVMHeaders.h', u'Source/WTF/wtf/text/CString.h']" exit_code: 1 Source/JavaScriptCore/llvm/LLVMAPI.h:36: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:125: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:126: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:127: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:128: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:129: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:130: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:131: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:132: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:133: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:134: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:135: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:136: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:137: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:138: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:139: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:140: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:141: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:142: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:143: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:144: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:145: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:146: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:147: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:148: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:149: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:150: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:151: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:152: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:153: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:154: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:155: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:156: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:157: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:158: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:159: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:160: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:161: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:162: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:163: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:164: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:165: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:166: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:167: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:168: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:169: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:170: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:171: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:172: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:173: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:174: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:175: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:176: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:177: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:178: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:179: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:180: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:181: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:182: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:183: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:184: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:185: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:186: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:187: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:188: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:189: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:190: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:191: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:192: Extra space after ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:491: Missing space after , [whitespace/comma] [3] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:559: Extra space after ( [whitespace/parens] [2] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:560: Extra space after ( [whitespace/parens] [2] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:579: Extra space after ( [whitespace/parens] [2] Source/JavaScriptCore/disassembler/LLVMDisassembler.cpp:33: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/llvm/LLVMHeaders.h:48: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 75 in 35 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 213834 [details] the patch Attachment 213834 [details] did not pass efl-ews (efl): Output: http://webkit-queues.appspot.com/results/3837028

Comment on attachment 213834 [details] the patch Attachment 213834 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/3836036

Comment on attachment 213834 [details] the patch 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

Comment on attachment 213834 [details] the patch Attachment 213834 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/3769049

Comment on attachment 213834 [details] the patch Attachment 213834 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/3836033

Comment on attachment 213834 [details] the patch Attachment 213834 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/3835039

Comment on attachment 213834 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=213834&action=review > Source/JavaScriptCore/config.h:57 > +#if defined(__cplusplus) && !defined(WTF_SUPPRESS_FAST_MALLOC) Why is the LLVM stuff using this header at all? Can't it have its own config.h that just pulls in Platform.h if that's all it needs? > Source/JavaScriptCore/jsc.cpp:849 > - RefPtr<VM> vm = VM::create(LargeHeap); > + VM* vm = VM::create(LargeHeap).leakRef(); This all looks unrelated to the rest of the patch? > Source/JavaScriptCore/Configurations/LLVMForJSC.xcconfig:1 > +// Copyright (C) 2009 Apple Inc. All rights reserved. You're living in the past! > Source/JavaScriptCore/Configurations/LLVMForJSC.xcconfig:40 > +HEADER_SEARCH_PATHS = "${BUILT_PRODUCTS_DIR}/DerivedSources/JavaScriptCore" $(HEADER_SEARCH_PATHS); Why is it necessary to have DerivedSources/JavaScriptCore on the header search path? If it is truly necessary, BUILT_PRODUCTS_DIR should be referenced using $() rather than ${}. > Source/JavaScriptCore/Configurations/LLVMForJSC.xcconfig:42 > +PRODUCT_NAME = libllvmForJSC; The product name should be just llvmForJSC. Settting EXECUTABLE_PREFIX set to "lib" will result in the binary having the correct name. > Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:4853 > + shellScript = "if [[ $ENABLE_FTL_JIT != \"ENABLE_FTL_JIT\" ]]\nthen\n exit 0\nfi\n\n# Copy the llvmForJSC library into the framework.\nditto \"${BUILT_PRODUCTS_DIR}/libllvmForJSC.dylib\" \"${BUILT_PRODUCTS_DIR}/JavaScriptCore.framework/Versions/A/Libraries/libllvmForJSC.dylib\"\nln -fs \"${BUILT_PRODUCTS_DIR}/JavaScriptCore.framework/Versions/A/Libraries\" \"${BUILT_PRODUCTS_DIR}/JavaScriptCore.framework/Libraries\""; The symlink should point to Versions/Current rather than Versions/A. The destination of the symlink should also be a relative path rather than an absolute one. You can also take advantage of Xcode configuration settings like CONTENTS_FOLDER_PATH and WRAPPER_NAME to avoid hard-coding JavaScriptCore.framework/Versions/A and JavaScriptCore.framework in the script. It may also be worth skipping the symlink step when SHALLOW_BUNDLE is equal to YES, since in that case the Versions hierarchy isn't used. > Source/JavaScriptCore/llvm/InitializeLLVM.cpp:31 > +#include <wtf/Threading.h> It doesn't look like you use anything from wtf/Threading.h. > Source/JavaScriptCore/llvm/InitializeLLVMMac.mm:34 > +#import <wtf/DataLog.h> > +#import <wtf/StringPrintStream.h> These don't seem necessary. > Source/JavaScriptCore/llvm/InitializeLLVMMac.mm:37 > +@interface JSCLLVMDummy : NSObject I'd name this something like JavaScriptCoreBundleFinder to follow WebCore's lead. > Source/JavaScriptCore/llvm/InitializeLLVMMac.mm:41 > +@implementation JSCLLVMDummy { > +} These braces are not necessary. > Source/JavaScriptCore/llvm/InitializeLLVMMac.mm:51 > + NSBundle* myBundle = [NSBundle bundleForClass:[JSCLLVMDummy class]]; > + NSString* path = [myBundle bundlePath]; > + > + initializeLLVMPOSIX(toCString([path UTF8String], "/Libraries/libllvmForJSC.dylib").data()); Your *'s are in the wrong place for Objective-C types. It seems weird to bother with toCString here when you could just build up the entire path as an NSString: NSString *bundle = [NSBundle bundleForClass:[JavaScriptCoreBundleFinder class]]; NSString *path = [[bundle bundlePath] stringByAppendingPathComponent:@"Libraries/libllvmForJSC.dylib"]; initializeLLVMPOSIX([path UTF8String]); > Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp:44 > + InitializerFunction initializer = bitwise_cast<InitializerFunction>( > + dlsym(library, "initializeAndGetJSCLLVMAPI")); I'm not sure it's necessary to wrap things like this. I think you've been staring at LLVM code for too long :) > Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:31 > +#define FOR_EACH_LLVM_API_FUNCTION(macro) \ There are a lot of style issues pointed out by the style checker that it'd be good to address. > Source/JavaScriptCore/llvm/LLVMHeaders.h:37 > +#undef __STDC_LIMIT_MACROS > +#undef __STDC_CONSTANT_MACROS > +#define __STDC_LIMIT_MACROS > +#define __STDC_CONSTANT_MACROS Should we be resetting these macros to their original state at the end of the header?

(In reply to comment #11) > (From update of attachment 213834 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=213834&action=review > > > Source/JavaScriptCore/config.h:57 > > +#if defined(__cplusplus) && !defined(WTF_SUPPRESS_FAST_MALLOC) > > Why is the LLVM stuff using this header at all? Can't it have its own config.h that just pulls in Platform.h if that's all it needs? I'll create a config_llvm.h. > > > Source/JavaScriptCore/jsc.cpp:849 > > - RefPtr<VM> vm = VM::create(LargeHeap); > > + VM* vm = VM::create(LargeHeap).leakRef(); > > This all looks unrelated to the rest of the patch? It's related. The jsc command-line normally works the way that this patch makes it work: it leaks a VM. We changed it to not leak a VM to work around exit-time destructors on LLVM: we would force a tidy VM shutdown to ensure that the FTL compiler thread wasn't running while LLVM's exit-time destructors ran. This patch fixes exit-time destructors and static initializers in LLVM by doing soft linking. Hence we can get rid of the hack in jsc. > > > Source/JavaScriptCore/Configurations/LLVMForJSC.xcconfig:1 > > +// Copyright (C) 2009 Apple Inc. All rights reserved. > > You're living in the past! Oops. > > > Source/JavaScriptCore/Configurations/LLVMForJSC.xcconfig:40 > > +HEADER_SEARCH_PATHS = "${BUILT_PRODUCTS_DIR}/DerivedSources/JavaScriptCore" $(HEADER_SEARCH_PATHS); > > Why is it necessary to have DerivedSources/JavaScriptCore on the header search path? If it is truly necessary, BUILT_PRODUCTS_DIR should be referenced using $() rather than ${}. It is necessary because JavaScriptCore.framework and llvmForJSC.dylib share the LLVMAPI.h, LLVMAPIFunctions.h, and LLVMHeaders.h files. As for using $() versus ${}, I was just doing what the other xcconfig's do. > > > Source/JavaScriptCore/Configurations/LLVMForJSC.xcconfig:42 > > +PRODUCT_NAME = libllvmForJSC; > > The product name should be just llvmForJSC. Settting EXECUTABLE_PREFIX set to "lib" will result in the binary having the correct name. Interesting. Xcode's default build settings used PRODUCT_NAME=libllvmForJSC, hence that's what I did here! I can change to what you suggest, though. > > > Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:4853 > > + shellScript = "if [[ $ENABLE_FTL_JIT != \"ENABLE_FTL_JIT\" ]]\nthen\n exit 0\nfi\n\n# Copy the llvmForJSC library into the framework.\nditto \"${BUILT_PRODUCTS_DIR}/libllvmForJSC.dylib\" \"${BUILT_PRODUCTS_DIR}/JavaScriptCore.framework/Versions/A/Libraries/libllvmForJSC.dylib\"\nln -fs \"${BUILT_PRODUCTS_DIR}/JavaScriptCore.framework/Versions/A/Libraries\" \"${BUILT_PRODUCTS_DIR}/JavaScriptCore.framework/Libraries\""; > > The symlink should point to Versions/Current rather than Versions/A. The destination of the symlink should also be a relative path rather than an absolute one. You can also take advantage of Xcode configuration settings like CONTENTS_FOLDER_PATH and WRAPPER_NAME to avoid hard-coding JavaScriptCore.framework/Versions/A and JavaScriptCore.framework in the script. OK, I'll investigate this. > > It may also be worth skipping the symlink step when SHALLOW_BUNDLE is equal to YES, since in that case the Versions hierarchy isn't used. Hmmm, that's an iOS only thing, right? Seems best to skip this until we are building on iOS and can actually test it. > > > Source/JavaScriptCore/llvm/InitializeLLVM.cpp:31 > > +#include <wtf/Threading.h> > > It doesn't look like you use anything from wtf/Threading.h. pthread_once(). > > > Source/JavaScriptCore/llvm/InitializeLLVMMac.mm:34 > > +#import <wtf/DataLog.h> > > +#import <wtf/StringPrintStream.h> > > These don't seem necessary. StringPrintStream gives you toCString(), which this uses. But I'll follow your suggestion and use NSString's APIs. > > > Source/JavaScriptCore/llvm/InitializeLLVMMac.mm:37 > > +@interface JSCLLVMDummy : NSObject > > I'd name this something like JavaScriptCoreBundleFinder to follow WebCore's lead. OK. > > > Source/JavaScriptCore/llvm/InitializeLLVMMac.mm:41 > > +@implementation JSCLLVMDummy { > > +} > > These braces are not necessary. Ok, I'll change it to: @implementation Blah @end For appropriate <Blah>. > > > Source/JavaScriptCore/llvm/InitializeLLVMMac.mm:51 > > + NSBundle* myBundle = [NSBundle bundleForClass:[JSCLLVMDummy class]]; > > + NSString* path = [myBundle bundlePath]; > > + > > + initializeLLVMPOSIX(toCString([path UTF8String], "/Libraries/libllvmForJSC.dylib").data()); > > Your *'s are in the wrong place for Objective-C types. It seems weird to bother with toCString here when you could just build up the entire path as an NSString: > > NSString *bundle = [NSBundle bundleForClass:[JavaScriptCoreBundleFinder class]]; > NSString *path = [[bundle bundlePath] stringByAppendingPathComponent:@"Libraries/libllvmForJSC.dylib"]; > > initializeLLVMPOSIX([path UTF8String]); Groovy, I'll do that. > > > Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp:44 > > + InitializerFunction initializer = bitwise_cast<InitializerFunction>( > > + dlsym(library, "initializeAndGetJSCLLVMAPI")); > > I'm not sure it's necessary to wrap things like this. I think you've been staring at LLVM code for too long :) I always wrap things because that's just how I roll. > > > Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:31 > > +#define FOR_EACH_LLVM_API_FUNCTION(macro) \ > > There are a lot of style issues pointed out by the style checker that it'd be good to address. :-/ This part was auto-generated. I'll address most of the issues but I'm not sure to what extent it's worthwhile to be strict about this. For example, LLVM uses a different parameter naming convention than we do and for this part it makes sense to just follow what the LLVM headers actually did rather than forcing WebKit convention. > > > Source/JavaScriptCore/llvm/LLVMHeaders.h:37 > > +#undef __STDC_LIMIT_MACROS > > +#undef __STDC_CONSTANT_MACROS > > +#define __STDC_LIMIT_MACROS > > +#define __STDC_CONSTANT_MACROS > > Should we be resetting these macros to their original state at the end of the header? That's not a bad idea. OTOH, these macros just mean that you get more C-isms; we don't use those C-isms (like INT64_C(...)) but having them defined doesn't actually hurt. OTOH, I wouldn't mind having those C-isms available in WebKit; I've definitely written some terrible code in the past just to get the equivalent of the C limit macros.

About to upload a new patch, comments below indicating what I fixed or didn't fix. (In reply to comment #12) > (In reply to comment #11) > > (From update of attachment 213834 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=213834&action=review > > > > > Source/JavaScriptCore/config.h:57 > > > +#if defined(__cplusplus) && !defined(WTF_SUPPRESS_FAST_MALLOC) > > > > Why is the LLVM stuff using this header at all? Can't it have its own config.h that just pulls in Platform.h if that's all it needs? > > I'll create a config_llvm.h. Fixed. > > > > > > Source/JavaScriptCore/jsc.cpp:849 > > > - RefPtr<VM> vm = VM::create(LargeHeap); > > > + VM* vm = VM::create(LargeHeap).leakRef(); > > > > This all looks unrelated to the rest of the patch? > > It's related. The jsc command-line normally works the way that this patch makes it work: it leaks a VM. We changed it to not leak a VM to work around exit-time destructors on LLVM: we would force a tidy VM shutdown to ensure that the FTL compiler thread wasn't running while LLVM's exit-time destructors ran. > > This patch fixes exit-time destructors and static initializers in LLVM by doing soft linking. Hence we can get rid of the hack in jsc. I kept this. It's related. > > > > > > Source/JavaScriptCore/Configurations/LLVMForJSC.xcconfig:1 > > > +// Copyright (C) 2009 Apple Inc. All rights reserved. > > > > You're living in the past! > > Oops. Fixed. > > > > > > Source/JavaScriptCore/Configurations/LLVMForJSC.xcconfig:40 > > > +HEADER_SEARCH_PATHS = "${BUILT_PRODUCTS_DIR}/DerivedSources/JavaScriptCore" $(HEADER_SEARCH_PATHS); > > > > Why is it necessary to have DerivedSources/JavaScriptCore on the header search path? If it is truly necessary, BUILT_PRODUCTS_DIR should be referenced using $() rather than ${}. > > It is necessary because JavaScriptCore.framework and llvmForJSC.dylib share the LLVMAPI.h, LLVMAPIFunctions.h, and LLVMHeaders.h files. > > As for using $() versus ${}, I was just doing what the other xcconfig's do. Changed to use $(). There are still a bunch of places where our xcconfig's use ${}; I'm not sure I understand what the difference is or which of those we should change. > > > > > > Source/JavaScriptCore/Configurations/LLVMForJSC.xcconfig:42 > > > +PRODUCT_NAME = libllvmForJSC; > > > > The product name should be just llvmForJSC. Settting EXECUTABLE_PREFIX set to "lib" will result in the binary having the correct name. > > Interesting. Xcode's default build settings used PRODUCT_NAME=libllvmForJSC, hence that's what I did here! I can change to what you suggest, though. Fixed. > > > > > > Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:4853 > > > + shellScript = "if [[ $ENABLE_FTL_JIT != \"ENABLE_FTL_JIT\" ]]\nthen\n exit 0\nfi\n\n# Copy the llvmForJSC library into the framework.\nditto \"${BUILT_PRODUCTS_DIR}/libllvmForJSC.dylib\" \"${BUILT_PRODUCTS_DIR}/JavaScriptCore.framework/Versions/A/Libraries/libllvmForJSC.dylib\"\nln -fs \"${BUILT_PRODUCTS_DIR}/JavaScriptCore.framework/Versions/A/Libraries\" \"${BUILT_PRODUCTS_DIR}/JavaScriptCore.framework/Libraries\""; > > > > The symlink should point to Versions/Current rather than Versions/A. The destination of the symlink should also be a relative path rather than an absolute one. You can also take advantage of Xcode configuration settings like CONTENTS_FOLDER_PATH and WRAPPER_NAME to avoid hard-coding JavaScriptCore.framework/Versions/A and JavaScriptCore.framework in the script. > > OK, I'll investigate this. Fixed. > > > > > It may also be worth skipping the symlink step when SHALLOW_BUNDLE is equal to YES, since in that case the Versions hierarchy isn't used. > > Hmmm, that's an iOS only thing, right? Seems best to skip this until we are building on iOS and can actually test it. I didn't do anything about this because I don't see how to test it right now. > > > > > > Source/JavaScriptCore/llvm/InitializeLLVM.cpp:31 > > > +#include <wtf/Threading.h> > > > > It doesn't look like you use anything from wtf/Threading.h. > > pthread_once(). Kept this the same. > > > > > > Source/JavaScriptCore/llvm/InitializeLLVMMac.mm:34 > > > +#import <wtf/DataLog.h> > > > +#import <wtf/StringPrintStream.h> > > > > These don't seem necessary. > > StringPrintStream gives you toCString(), which this uses. But I'll follow your suggestion and use NSString's APIs. Fixed. > > > > > > Source/JavaScriptCore/llvm/InitializeLLVMMac.mm:37 > > > +@interface JSCLLVMDummy : NSObject > > > > I'd name this something like JavaScriptCoreBundleFinder to follow WebCore's lead. > > OK. Changed it to JSJavaScriptCoreBundleFinder. There's some script that checks for the JS prefix in all ObjC classes in JSC.framework, so I decided to appease that script. > > > > > > Source/JavaScriptCore/llvm/InitializeLLVMMac.mm:41 > > > +@implementation JSCLLVMDummy { > > > +} > > > > These braces are not necessary. > > Ok, I'll change it to: > > @implementation Blah > @end > > For appropriate <Blah>. Fixed. > > > > > > Source/JavaScriptCore/llvm/InitializeLLVMMac.mm:51 > > > + NSBundle* myBundle = [NSBundle bundleForClass:[JSCLLVMDummy class]]; > > > + NSString* path = [myBundle bundlePath]; > > > + > > > + initializeLLVMPOSIX(toCString([path UTF8String], "/Libraries/libllvmForJSC.dylib").data()); > > > > Your *'s are in the wrong place for Objective-C types. It seems weird to bother with toCString here when you could just build up the entire path as an NSString: Fixed. > > > > NSString *bundle = [NSBundle bundleForClass:[JavaScriptCoreBundleFinder class]]; > > NSString *path = [[bundle bundlePath] stringByAppendingPathComponent:@"Libraries/libllvmForJSC.dylib"]; > > > > initializeLLVMPOSIX([path UTF8String]); > > Groovy, I'll do that. Fixed. > > > > > > Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp:44 > > > + InitializerFunction initializer = bitwise_cast<InitializerFunction>( > > > + dlsym(library, "initializeAndGetJSCLLVMAPI")); > > > > I'm not sure it's necessary to wrap things like this. I think you've been staring at LLVM code for too long :) > > I always wrap things because that's just how I roll. Kept it as is. > > > > > > Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:31 > > > +#define FOR_EACH_LLVM_API_FUNCTION(macro) \ > > > > There are a lot of style issues pointed out by the style checker that it'd be good to address. > > :-/ > > This part was auto-generated. I'll address most of the issues but I'm not sure to what extent it's worthwhile to be strict about this. For example, LLVM uses a different parameter naming convention than we do and for this part it makes sense to just follow what the LLVM headers actually did rather than forcing WebKit convention. Took care of obvious style faux pas but that list continues to use LLVM style conventions for these LLVM signatures. I think it's the best way to go. > > > > > > Source/JavaScriptCore/llvm/LLVMHeaders.h:37 > > > +#undef __STDC_LIMIT_MACROS > > > +#undef __STDC_CONSTANT_MACROS > > > +#define __STDC_LIMIT_MACROS > > > +#define __STDC_CONSTANT_MACROS > > > > Should we be resetting these macros to their original state at the end of the header? > > That's not a bad idea. OTOH, these macros just mean that you get more C-isms; we don't use those C-isms (like INT64_C(...)) but having them defined doesn't actually hurt. OTOH, I wouldn't mind having those C-isms available in WebKit; I've definitely written some terrible code in the past just to get the equivalent of the C limit macros. Put some #undef's in there. Also removed the #undef's at the top since it would be a bug for WebKit to have already defined those macros.

Created attachment 213842 [details] the patch

Created attachment 213843 [details] the patch Fix more style.

Comment on attachment 213843 [details] the patch Never mind, this still needs some love.

Attachment 213843 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig', u'Source/JavaScriptCore/Configurations/LLVMForJSC.xcconfig', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/config.h', u'Source/JavaScriptCore/dfg/DFGPlan.cpp', u'Source/JavaScriptCore/disassembler/LLVMDisassembler.cpp', u'Source/JavaScriptCore/ftl/FTLAbbreviatedTypes.h', u'Source/JavaScriptCore/ftl/FTLAbbreviations.h', u'Source/JavaScriptCore/ftl/FTLCompile.cpp', u'Source/JavaScriptCore/ftl/FTLFail.cpp', u'Source/JavaScriptCore/ftl/FTLJITCode.h', u'Source/JavaScriptCore/ftl/FTLJITFinalizer.h', u'Source/JavaScriptCore/ftl/FTLLink.cpp', u'Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.h', u'Source/JavaScriptCore/ftl/FTLState.cpp', u'Source/JavaScriptCore/ftl/WebKitLLVMLibraryAnchor.cpp', u'Source/JavaScriptCore/jsc.cpp', u'Source/JavaScriptCore/llvm/InitializeLLVM.cpp', u'Source/JavaScriptCore/llvm/InitializeLLVM.h', u'Source/JavaScriptCore/llvm/InitializeLLVMMac.mm', u'Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp', u'Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.h', u'Source/JavaScriptCore/llvm/LLVMAPI.cpp', u'Source/JavaScriptCore/llvm/LLVMAPI.h', u'Source/JavaScriptCore/llvm/LLVMAPIFunctions.h', u'Source/JavaScriptCore/llvm/LLVMHeaders.h', u'Source/JavaScriptCore/llvm/library/LLVMAnchor.cpp', u'Source/JavaScriptCore/llvm/library/LLVMExports.cpp', u'Source/JavaScriptCore/llvm/library/LLVMOverrides.cpp', u'Source/JavaScriptCore/llvm/library/config_llvm.h', u'Source/JavaScriptCore/runtime/InitializeThreading.cpp', u'Source/JavaScriptCore/runtime/Options.h', u'Source/WTF/ChangeLog', u'Source/WTF/WTF.xcodeproj/project.pbxproj', u'Source/WTF/wtf/LLVMHeaders.h', u'Source/WTF/wtf/text/CString.h']" exit_code: 1 Source/JavaScriptCore/llvm/LLVMAPI.h:36: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/library/config_llvm.h:26: Alphabetical sorting problem. [build/include_order] [4] Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:491: Missing space after , [whitespace/comma] [3] Total errors found: 3 in 36 files If any of these errors are false positives, please file a bug against check-webkit-style.

(In reply to comment #7) > (From update of attachment 213834 [details]) > 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? That's what the jsc shell used to do before I added a hack to not leak it. I prefer for the jsc shell to leak the VM since that's what the browser - and probably most JSC clients - will do. > > >> 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

Created attachment 213844 [details] the patch OK, now I think it's ready.

Attachment 213844 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig', u'Source/JavaScriptCore/Configurations/LLVMForJSC.xcconfig', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/dfg/DFGPlan.cpp', u'Source/JavaScriptCore/disassembler/LLVMDisassembler.cpp', u'Source/JavaScriptCore/ftl/FTLAbbreviatedTypes.h', u'Source/JavaScriptCore/ftl/FTLAbbreviations.h', u'Source/JavaScriptCore/ftl/FTLCompile.cpp', u'Source/JavaScriptCore/ftl/FTLFail.cpp', u'Source/JavaScriptCore/ftl/FTLJITCode.h', u'Source/JavaScriptCore/ftl/FTLJITFinalizer.h', u'Source/JavaScriptCore/ftl/FTLLink.cpp', u'Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.h', u'Source/JavaScriptCore/ftl/FTLState.cpp', u'Source/JavaScriptCore/ftl/WebKitLLVMLibraryAnchor.cpp', u'Source/JavaScriptCore/jsc.cpp', u'Source/JavaScriptCore/llvm/InitializeLLVM.cpp', u'Source/JavaScriptCore/llvm/InitializeLLVM.h', u'Source/JavaScriptCore/llvm/InitializeLLVMMac.mm', u'Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp', u'Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.h', u'Source/JavaScriptCore/llvm/LLVMAPI.cpp', u'Source/JavaScriptCore/llvm/LLVMAPI.h', u'Source/JavaScriptCore/llvm/LLVMAPIFunctions.h', u'Source/JavaScriptCore/llvm/LLVMHeaders.h', u'Source/JavaScriptCore/llvm/library/LLVMAnchor.cpp', u'Source/JavaScriptCore/llvm/library/LLVMExports.cpp', u'Source/JavaScriptCore/llvm/library/LLVMOverrides.cpp', u'Source/JavaScriptCore/llvm/library/config_llvm.h', u'Source/JavaScriptCore/runtime/InitializeThreading.cpp', u'Source/JavaScriptCore/runtime/Options.h', u'Source/WTF/ChangeLog', u'Source/WTF/WTF.xcodeproj/project.pbxproj', u'Source/WTF/wtf/LLVMHeaders.h', u'Source/WTF/wtf/text/CString.h']" exit_code: 1 Source/JavaScriptCore/llvm/LLVMAPI.h:40: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/library/config_llvm.h:26: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 35 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 213845 [details] the patch Fixed a goof in my use of ln -fs. We were getting a Libraries/Libraries symlink. This patch won't create a symlink if Libraries already exists.

Attachment 213845 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig', u'Source/JavaScriptCore/Configurations/LLVMForJSC.xcconfig', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/dfg/DFGPlan.cpp', u'Source/JavaScriptCore/disassembler/LLVMDisassembler.cpp', u'Source/JavaScriptCore/ftl/FTLAbbreviatedTypes.h', u'Source/JavaScriptCore/ftl/FTLAbbreviations.h', u'Source/JavaScriptCore/ftl/FTLCompile.cpp', u'Source/JavaScriptCore/ftl/FTLFail.cpp', u'Source/JavaScriptCore/ftl/FTLJITCode.h', u'Source/JavaScriptCore/ftl/FTLJITFinalizer.h', u'Source/JavaScriptCore/ftl/FTLLink.cpp', u'Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.h', u'Source/JavaScriptCore/ftl/FTLState.cpp', u'Source/JavaScriptCore/ftl/WebKitLLVMLibraryAnchor.cpp', u'Source/JavaScriptCore/jsc.cpp', u'Source/JavaScriptCore/llvm/InitializeLLVM.cpp', u'Source/JavaScriptCore/llvm/InitializeLLVM.h', u'Source/JavaScriptCore/llvm/InitializeLLVMMac.mm', u'Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp', u'Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.h', u'Source/JavaScriptCore/llvm/LLVMAPI.cpp', u'Source/JavaScriptCore/llvm/LLVMAPI.h', u'Source/JavaScriptCore/llvm/LLVMAPIFunctions.h', u'Source/JavaScriptCore/llvm/LLVMHeaders.h', u'Source/JavaScriptCore/llvm/library/LLVMAnchor.cpp', u'Source/JavaScriptCore/llvm/library/LLVMExports.cpp', u'Source/JavaScriptCore/llvm/library/LLVMOverrides.cpp', u'Source/JavaScriptCore/llvm/library/config_llvm.h', u'Source/JavaScriptCore/runtime/InitializeThreading.cpp', u'Source/JavaScriptCore/runtime/Options.h', u'Source/WTF/ChangeLog', u'Source/WTF/WTF.xcodeproj/project.pbxproj', u'Source/WTF/wtf/LLVMHeaders.h', u'Source/WTF/wtf/text/CString.h']" exit_code: 1 Source/JavaScriptCore/llvm/LLVMAPI.h:40: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/library/config_llvm.h:26: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 35 files If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 213845 [details] the patch Attachment 213845 [details] did not pass win-ews (win): Output: http://webkit-queues.appspot.com/results/3580046

Created attachment 213887 [details] the patch Hopefully this breaks less things.

Looks good to me.

(In reply to comment #12) > This patch fixes exit-time destructors and static initializers in LLVM by doing soft linking. Hence we can get rid of the hack in jsc. Ah, cool! > As for using $() versus ${}, I was just doing what the other xcconfig's do. It looks like this mistake crept in to one place in one .xcconfig file long ago and has since spread to others. The fact that ${} works at all isn't something that's documented AFAICT. > Hmmm, that's an iOS only thing, right? Seems best to skip this until we are building on iOS and can actually test it. Ok, sounds reasonable. > > > Source/JavaScriptCore/llvm/InitializeLLVM.cpp:31 > > > +#include <wtf/Threading.h> > > > > It doesn't look like you use anything from wtf/Threading.h. > > pthread_once(). That comes from pthread.h. You should just include it directly. > > > > > Source/JavaScriptCore/llvm/LLVMAPIFunctions.h:31 > > > +#define FOR_EACH_LLVM_API_FUNCTION(macro) \ > > > > There are a lot of style issues pointed out by the style checker that it'd be good to address. > > :-/ > > This part was auto-generated. I'll address most of the issues but I'm not sure to what extent it's worthwhile to be strict about this. For example, LLVM uses a different parameter naming convention than we do and for this part it makes sense to just follow what the LLVM headers actually did rather than forcing WebKit convention. I'm not too concerned about whether you address the naming issues, since as you mention it makes sense for this to be consistent with LLVM. A lot of warnings were about inconsistent whitespace. I think you've fixed that in your latest patch though.

Comment on attachment 213887 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=213887&action=review > Source/JavaScriptCore/llvm/InitializeLLVM.cpp:31 > +#include <wtf/Threading.h> This should be #include <pthread.h>

(In reply to comment #27) > (From update of attachment 213887 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=213887&action=review > > > Source/JavaScriptCore/llvm/InitializeLLVM.cpp:31 > > +#include <wtf/Threading.h> > > This should be #include <pthread.h> Yup, I'll fix. Thanks for the reviews!

Created attachment 213912 [details] patch for ews Should be all done, but I want to see what the bots say.

Attachment 213912 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/Configurations/JavaScriptCore.xcconfig', u'Source/JavaScriptCore/Configurations/LLVMForJSC.xcconfig', u'Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj', u'Source/JavaScriptCore/dfg/DFGPlan.cpp', u'Source/JavaScriptCore/disassembler/LLVMDisassembler.cpp', u'Source/JavaScriptCore/ftl/FTLAbbreviatedTypes.h', u'Source/JavaScriptCore/ftl/FTLAbbreviations.h', u'Source/JavaScriptCore/ftl/FTLCompile.cpp', u'Source/JavaScriptCore/ftl/FTLFail.cpp', u'Source/JavaScriptCore/ftl/FTLJITCode.h', u'Source/JavaScriptCore/ftl/FTLJITFinalizer.h', u'Source/JavaScriptCore/ftl/FTLLink.cpp', u'Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.cpp', u'Source/JavaScriptCore/ftl/FTLOutput.h', u'Source/JavaScriptCore/ftl/FTLState.cpp', u'Source/JavaScriptCore/ftl/WebKitLLVMLibraryAnchor.cpp', u'Source/JavaScriptCore/jsc.cpp', u'Source/JavaScriptCore/llvm/InitializeLLVM.cpp', u'Source/JavaScriptCore/llvm/InitializeLLVM.h', u'Source/JavaScriptCore/llvm/InitializeLLVMMac.mm', u'Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp', u'Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.h', u'Source/JavaScriptCore/llvm/LLVMAPI.cpp', u'Source/JavaScriptCore/llvm/LLVMAPI.h', u'Source/JavaScriptCore/llvm/LLVMAPIFunctions.h', u'Source/JavaScriptCore/llvm/LLVMHeaders.h', u'Source/JavaScriptCore/llvm/library/LLVMAnchor.cpp', u'Source/JavaScriptCore/llvm/library/LLVMExports.cpp', u'Source/JavaScriptCore/llvm/library/LLVMOverrides.cpp', u'Source/JavaScriptCore/llvm/library/config_llvm.h', u'Source/JavaScriptCore/runtime/InitializeThreading.cpp', u'Source/JavaScriptCore/runtime/Options.h', u'Source/WTF/ChangeLog', u'Source/WTF/WTF.xcodeproj/project.pbxproj', u'Source/WTF/wtf/LLVMHeaders.h', u'Source/WTF/wtf/text/CString.h', u'Tools/ChangeLog', u'Tools/Scripts/configure-llvm']" exit_code: 1 Source/JavaScriptCore/llvm/LLVMAPI.h:40: Extra space before ( in function call [whitespace/parens] [4] Source/JavaScriptCore/llvm/library/config_llvm.h:26: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 2 in 36 files If any of these errors are false positives, please file a bug against check-webkit-style.

Landed in http://trac.webkit.org/changeset/157260