Bug 190514 – [Win][Clang] Do not give -Wall to clang-cl because it is treated as -Weverything

• Status: RESOLVED
• Resolution: FIXED
• Component: CMake

[Fujii Hironori]

[Win][Clang] Do not give -Wall to clang-cl because it is treated as -Weverything

According to clang-cl manual, it has following command switch mapping:

https://clang.llvm.org/docs/UsersManual.html#id8

>  /W4                     Enable -Wall and -Wextra
>  /Wall                   Enable -Weverything

As the result, clang-cl treat -Wall as -Weverything, and reports ton of warning messages.
reported

[Fujii Hironori]

Created attachment 352155 [details]
Patch

[Fujii Hironori]

There is another issue some switches aren't effective.
For exampel -Wno-unknown-pragmas.
-Wno-unknown-pragmas should be specified after /W4, not before.

[Fujii Hironori]

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

[Fujii Hironori]

Created attachment 352305 [details]
Patch

[Fujii Hironori]

Created attachment 352306 [details]
Patch

[Fujii Hironori]

Created attachment 352307 [details]
Patch

[Konstantin Tokarev]

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

[Fujii Hironori]

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.

[Fujii Hironori]

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.

[Fujii Hironori]

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/

[Fujii Hironori]

Created attachment 352687 [details]
Patch

[Fujii Hironori]

Review, please.

[Michael Catanzaro]

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.)

[Fujii Hironori]

Created attachment 352757 [details]
Patch for landing

Thank you for the review. It makes sense. Applied the review feedback.

[Fujii Hironori]

Comment on attachment 352757 [details]
Patch for landing

Clearing flags on attachment: 352757

Committed r237282: <https://trac.webkit.org/changeset/237282>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.