Summary: | [Win][Clang] Do not give -Wall to clang-cl because it is treated as -Weverything | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Fujii Hironori <Hironori.Fujii> | ||||||||||||||
Component: | CMake | Assignee: | Fujii Hironori <Hironori.Fujii> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | achristensen, annulen, bfulgham, mcatanzaro, pvollan, webkit-bug-importer | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 171618 | ||||||||||||||||
Attachments: |
|
Description
Fujii Hironori
2018-10-12 03:37:02 PDT
Created attachment 352155 [details]
Patch
There is another issue some switches aren't effective. For exampel -Wno-unknown-pragmas. -Wno-unknown-pragmas should be specified after /W4, not before. GCC and Clang treat -Wall differently. Clang processes all -W* options in left-to-right order. GCC processes -Wall and -Wextra options first. https://bugs.llvm.org/show_bug.cgi?id=24376 https://gist.github.com/fujii/afb137300be40c71ea61b28391035ce7 Created attachment 352305 [details]
Patch
Created attachment 352306 [details]
Patch
Created attachment 352307 [details]
Patch
I think it would be more reasonable to avoid feeding COMPILER_IS_GCC_OR_CLANG options for clang-cl, because it expects cl.exe options instead Thanks for your feedback. I was also thinking the same idea. But, I'm not confident which is better. I will try to create a patch in another bug ticket. clang-cl compiles almost same code paths with MSVC. But there is only one exception. clang-cl uses warning macros for Clang (IGNORE_WARNINGS_* and IGNORE_CLANG_WARNINGS_*), even though those macros are disabled for MSVC. https://github.com/WebKit/webkit/blob/947003f6e3e05f3387e625d862e18cf61c15f03d/Source/WTF/wtf/Compiler.h#L442 With regard to compilation warnings, clang-cl is closer to clang than MSVC. It turned out it is not simple as I expected. Here are all usage of COMPILER_IS_GCC_OR_CLANG. https://github.com/WebKit/webkit/search?q=COMPILER_IS_GCC_OR_CLANG&unscoped_q=COMPILER_IS_GCC_OR_CLANG Almost all usage of COMPILER_IS_GCC_OR_CLANG is to suppress compilation warnings, such like the following: https://github.com/WebKit/webkit/blob/8138e195f0da39b5a476cbab5d3f1b8aafb6be4e/Tools/TestWebKitAPI/CMakeLists.txt#L265 > if (COMPILER_IS_GCC_OR_CLANG) > WEBKIT_ADD_TARGET_CXX_FLAGS(TestWTF -Wno-dangling-else > -Wno-sign-compare > -Wno-undef > -Wno-unused-parameter) > endif () I'd like to share these warning switched with clang-cl. It seems COMPILER_IS_GCC_OR_CLANG should be enabled for clang-cl. Comment on attachment 352307 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=352307&action=review > Source/cmake/WebKitCompilerFlags.cmake:147 > + # -Wall and -Wextra should be speficied before -Wno-* for Clang. s/speficied/specified/ Created attachment 352687 [details]
Patch
Review, please. Comment on attachment 352687 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=352687&action=review > Source/cmake/WebKitCompilerFlags.cmake:148 > +if (NOT MSVC) What about: if (COMPILER_IS_GCC_OR_CLANG AND NOT MSVC) It's just unfriendly to pass GCC flags to random compilers that might not be GCC compatible (even if it's unlikely that somebody would be attempting to use such a compiler nowadays.) Created attachment 352757 [details]
Patch for landing
Thank you for the review. It makes sense. Applied the review feedback.
Comment on attachment 352757 [details] Patch for landing Clearing flags on attachment: 352757 Committed r237282: <https://trac.webkit.org/changeset/237282> All reviewed patches have been landed. Closing bug. |