[cmake] Replace -Wall with /W4 in clang-cl options now that -Wall aliases -Weverything

[gbedwell]

Summary

We noticed that our Windows selfhost build was generating *incredibly* verbose warning output (thousands and thousands of -Wc++98-compat warnings for example). I'm pretty sure that this is down to r319116 which aliased clang-cl's -Wall to clang's -Weverything. As we were adding -Wall to clang-cl's options in our own CMake config we ended up getting -Weverything output.

Instead, reuse the code-path for cl.exe that adds /W4 instead, which for clang-cl alises clang's "-Wall -Wextra" which matches what clang-cl's /Wall previously aliased.

This should restore the verbosity of a Windows selfhost build back to its previous levels. I suspect that will fix the bot issues that Galina mentioned.

[rnk]

lgtm

This has the side effect of enabling -Wextra for Windows self-host builds. Are we sure we want to do that?

cmake/modules/HandleLLVMOptions.cmake

547 ↗

(On Diff #124748)

I'd say: Don't add -Wall for clang-cl, because it maps -Wall to -Weverything for MSVC compatibility.

[gbedwell]

In D40603#939278, @rnk wrote:

lgtm

This has the side effect of enabling -Wextra for Windows self-host builds. Are we sure we want to do that?

Thanks!

I think so. This is where this flag aliasing gets a bit messy! :)
Looking at the recent patch:

 def _SLASH_W4 : CLFlag<"W4">, HelpText<"Enable -Wall and -Wextra">, Alias<WCL4>;
-def _SLASH_Wall : CLFlag<"Wall">, HelpText<"Enable -Wall and -Wextra">, Alias<WCL4>;
+def _SLASH_Wall : CLFlag<"Wall">, HelpText<"Enable -Weverything">,

If the help text is to be believed, clang-cl's /Wall used to map to "-Wall -Wextra" anyway, so if I'm correct this should take us back to the old behaviour.

llvm/trunk/cmake/modules/HandleLLVMOptions.cmake

if (LLVM_ENABLE_MODULES)
     if (CXX_SUPPORTS_MODULES)
       append("${module_flags}" CMAKE_CXX_FLAGS)
     else()
       message(FATAL_ERROR "LLVM_ENABLE_MODULES is not supported by this compiler")
     endif()
   endif(LLVM_ENABLE_MODULES)
 endif( MSVC )
 
-if (MSVC AND NOT CLANG_CL)
+if (MSVC)
+  if (NOT CLANG_CL)
     set(msvc_warning_flags
       # Disabled warnings.
       -wd4141 # Suppress ''modifier' : used more than once' (because of __forceinline combined with inline)
       -wd4146 # Suppress 'unary minus operator applied to unsigned type, result still unsigned'
       -wd4180 # Suppress 'qualifier applied to function type has no meaning; ignored'
       -wd4244 # Suppress ''argument' : conversion from 'type1' to 'type2', possible loss of data'
       -wd4258 # Suppress ''var' : definition from the for loop is ignored; the definition from the enclosing scope is used'
       -wd4267 # Suppress ''var' : conversion from 'size_t' to 'type', possible loss of data'
       -wd4291 # Suppress ''declaration' : no matching operator delete found; memory will not be freed if initialization throws an exception'
       -wd4345 # Suppress 'behavior change: an object of POD type constructed with an initializer of the form () will be default-initialized'
       -wd4351 # Suppress 'new behavior: elements of array 'array' will be default initialized'
       -wd4355 # Suppress ''this' : used in base member initializer list'
       -wd4456 # Suppress 'declaration of 'var' hides local variable'
       -wd4457 # Suppress 'declaration of 'var' hides function parameter'
       -wd4458 # Suppress 'declaration of 'var' hides class member'
       -wd4459 # Suppress 'declaration of 'var' hides global declaration'
       -wd4503 # Suppress ''identifier' : decorated name length exceeded, name was truncated'
       -wd4624 # Suppress ''derived class' : destructor could not be generated because a base class destructor is inaccessible'
       -wd4722 # Suppress 'function' : destructor never returns, potential memory leak
       -wd4800 # Suppress ''type' : forcing value to bool 'true' or 'false' (performance warning)'
       -wd4100 # Suppress 'unreferenced formal parameter'
       -wd4127 # Suppress 'conditional expression is constant'
       -wd4512 # Suppress 'assignment operator could not be generated'
       -wd4505 # Suppress 'unreferenced local function has been removed'
       -wd4610 # Suppress '<class> can never be instantiated'
       -wd4510 # Suppress 'default constructor could not be generated'
       -wd4702 # Suppress 'unreachable code'
       -wd4245 # Suppress 'signed/unsigned mismatch'
       -wd4706 # Suppress 'assignment within conditional expression'
       -wd4310 # Suppress 'cast truncates constant value'
       -wd4701 # Suppress 'potentially uninitialized local variable'
       -wd4703 # Suppress 'potentially uninitialized local pointer variable'
       -wd4389 # Suppress 'signed/unsigned mismatch'
       -wd4611 # Suppress 'interaction between '_setjmp' and C++ object destruction is non-portable'
       -wd4805 # Suppress 'unsafe mix of type <type> and type <type> in operation'
       -wd4204 # Suppress 'nonstandard extension used : non-constant aggregate initializer'
       -wd4577 # Suppress 'noexcept used with no exception handling mode specified; termination on exception is not guaranteed'
       -wd4091 # Suppress 'typedef: ignored on left of '' when no variable is declared'
           # C4592 is disabled because of false positives in Visual Studio 2015
           # Update 1. Re-evaluate the usefulness of this diagnostic with Update 2.
       -wd4592 # Suppress ''var': symbol will be dynamically initialized (implementation limitation)
       -wd4319 # Suppress ''operator' : zero extending 'type' to 'type' of greater size'
 
       # Ideally, we'd like this warning to be enabled, but MSVC 2013 doesn't
       # support the 'aligned' attribute in the way that clang sources requires (for
       # any code that uses the LLVM_ALIGNAS macro), so this is must be disabled to
       # avoid unwanted alignment warnings.
       # When we switch to requiring a version of MSVC that supports the 'alignas'
       # specifier (MSVC 2015?) this warning can be re-enabled.
       -wd4324 # Suppress 'structure was padded due to __declspec(align())'
 
       # Promoted warnings.
       -w14062 # Promote 'enumerator in switch of enum is not handled' to level 1 warning.
 
       # Promoted warnings to errors.
       -we4238 # Promote 'nonstandard extension used : class rvalue used as lvalue' to error.
       )
+  endif(NOT CLANG_CL)
 
   # Enable warnings
   if (LLVM_ENABLE_WARNINGS)
     # Put /W4 in front of all the -we flags. cl.exe doesn't care, but for
     # clang-cl having /W4 after the -we flags will re-enable the warnings
     # disabled by -we.
     set(msvc_warning_flags "/W4 ${msvc_warning_flags}")
     # CMake appends /W3 by default, and having /W3 followed by /W4 will result in
     # cl : Command line warning D9025 : overriding '/W3' with '/W4'.  Since this is
     # a command line warning and not a compiler warning, it cannot be suppressed except
     # by fixing the command line.
     string(REGEX REPLACE " /W[0-4]" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
     string(REGEX REPLACE " /W[0-4]" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
 
     if (LLVM_ENABLE_PEDANTIC)
       # No MSVC equivalent available
     endif (LLVM_ENABLE_PEDANTIC)
   endif (LLVM_ENABLE_WARNINGS)
 
   foreach(flag ${msvc_warning_flags})
     append("${flag}" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
   endforeach(flag)
-endif (MSVC AND NOT CLANG_CL)
+endif (MSVC)
 
 if (LLVM_ENABLE_WARNINGS AND (LLVM_COMPILER_IS_GCC_COMPATIBLE OR CLANG_CL))
-  append("-Wall -W -Wno-unused-parameter -Wwrite-strings" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+
+  # Don't add -Wall for clang-cl, because it maps -Wall to -Weverything for
+  # MSVC compatibility.  /W4 is added above instead.
+  if (NOT CLANG_CL)
+    append("-Wall" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+  endif()
+
+  append("-W -Wno-unused-parameter -Wwrite-strings" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
   append("-Wcast-qual" CMAKE_CXX_FLAGS)
 
   # Turn off missing field initializer warnings for gcc to avoid noise from
   # false positives with empty {}. Turn them on otherwise (they're off by
   # default for clang).
   check_cxx_compiler_flag("-Wmissing-field-initializers" CXX_SUPPORTS_MISSING_FIELD_INITIALIZERS_FLAG)
   if (CXX_SUPPORTS_MISSING_FIELD_INITIALIZERS_FLAG)
     if (CMAKE_COMPILER_IS_GNUCXX)