Bug 133546

Created attachment 232547 [details] Patch

Comment on attachment 232547 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232547&action=review > Source/JavaScriptCore/ftl/FTLLocation.cpp:64 > + return loc; It would be far better to just say "return Location();" > Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp:66 > +#pragma GCC diagnostic ignored "-Wmissing-format-attribute" Why?

(In reply to comment #2) > (From update of attachment 232547 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=232547&action=review > > > Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp:66 > > +#pragma GCC diagnostic ignored "-Wmissing-format-attribute" > > Why? Because I don't know any better. This looks like false positive warning to me, but it breaks the build, so we have to ignore it. Do you have any better idea how to handle this warning?

Created attachment 232856 [details] Patch

Comment on attachment 232856 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=232856&action=review > Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp:66 > +#pragma GCC diagnostic ignored "-Wmissing-format-attribute" I think you should add the format attribute that it is asking for. Have you tried it? Also, it would be useful to post the compiler error you're getting so that others could suggest a better fix. In general using pragmas to disable warnings is not an OK way of fixing compiler warnings.

(In reply to comment #5) > (From update of attachment 232856 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=232856&action=review > > > Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp:66 > > +#pragma GCC diagnostic ignored "-Wmissing-format-attribute" > > I think you should add the format attribute that it is asking for. Have you tried it? Also, it would be useful to post the compiler error you're getting so that others could suggest a better fix. Ok the problem is the following. We have this logger function in WTF: void WTFLogAlwaysAndCrash(const char* format, ...) __attribute__((__format__(printf, 1, 2))) And we have this function in LLVMExports.cpp#56 that has a function pointer parameter: extern "C" WTF_EXPORT_PRIVATE JSC::LLVMAPI* initializeAndGetJSCLLVMAPI(void (*)(const char*, ...)); Then we call the function with a logger function: initializeAndGetJSCLLVMAPI(WTFLogAlwaysAndCrash); And we got the folloing waring message: /mnt/work/Webkit/Webkit/Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp:66:44: error: parameter might be a candidate for a format attribute [-Werror=suggest-attribute=format] /mnt/work/Webkit/Webkit/Source/JavaScriptCore/llvm/InitializeLLVMPOSIX.cpp:66:44: error: argument of function call might be a candidate for a format attribute [-Werror=suggest-attribute=format] This is a bit crazy because it points to the call of 'initializeAndGetJSCLLVMAPI' and not to the call of 'WTFLogAlwaysAndCrash'. We call the callback here: https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/llvm/library/LLVMExports.cpp#L70 You can see that is has format argument (and doesn't have any additional args). So it still looks like a false positive warning message for me. But if someone know a better way to resolve this warning, then please let me know.

Created attachment 233163 [details] Patch

Comment on attachment 233163 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=233163&action=review > Source/JavaScriptCore/ftl/FTLAbstractHeap.cpp:67 > + if ((static_cast<unsigned>(1) << i) == m_elementSize) { It’s much better to write this as 1U rather than static_cast<unsigned>(1). > Source/JavaScriptCore/ftl/FTLStackMaps.cpp:53 > + out.printf("0x%016llx", static_cast<long long unsigned>(integer)); We normally call the type "unsigned long long" rather than "long long unsigned".

Created attachment 233968 [details] patch for landing

Comment on attachment 233968 [details] patch for landing Clearing flags on attachment: 233968 Committed r170525: <http://trac.webkit.org/changeset/170525>

All reviewed patches have been landed. Closing bug.