[flang] Turn off FLANG_ENABLE_WERROR by default

[isuruf]

Summary

This is a follow up to https://reviews.llvm.org/D78306

[sscalpone]

These change worked in my environment.

[DavidTruby]

I support this change and have elaborated on why in https://reviews.llvm.org/D78306.
This patch also shows why having Werror on by default can be an issue.

[sscalpone]

@isuruf Are there some warning that cannot be removed with reasonable effort?

[mehdi_amini]

In D81695#2093046, @sscalpone wrote:

@isuruf Are there some warning that cannot be removed with reasonable effort?

I don't think this is about not willing to remove warnings, it is about what is the right default config.

Right for example now our MLIR bot does not show warnings with gcc-5 nor with clang-8, but for example clang-5 still wants extra braces:

warning: suggest braces around initialization of subobject [-Wmissing-braces]
 std::array<StringRef, 2> silentAttrNames{getIndexingMapsAttrName(),
                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~
                                          {

Having default to Werror may lead to broken build for users because we can't test every possible combination all the time.

[DavidTruby]

There is another example here of our BuildBots failing for very spurious warnings due to -Werror:
http://lab.llvm.org:8014/builders/flang-aarch64-ubuntu-clang/builds/229/steps/build-unified-tree/logs/stdio

[tskeith]

In D81695#2108974, @DavidTruby wrote:

There is another example here of our BuildBots failing for very spurious warnings due to -Werror:
http://lab.llvm.org:8014/builders/flang-aarch64-ubuntu-clang/builds/229/steps/build-unified-tree/logs/stdio

The warning is not spurious, so this example shows -Werror in the best possible light:
On the positive side, it detected a real error that was immediately fixed and might not have been noticed so soon if it were just a warning.
On the negative side, it prevented the rest of the build from happening and tests from being run, as happens with any compilation error.

[mehdi_amini]

In D81695#2093046, @sscalpone wrote:

@isuruf Are there some warning that cannot be removed with reasonable effort?

Since I'm just building with gcc-9 for some reason right now, here is a tough warning:

llvm-project/llvm/include/llvm/ExecutionEngine/Orc/RPC/RPCUtils.h:1534:27: warning: moving a local object in a return statement prevents copy elision [-Wpessimizing-move]
 1534 |       return std::move(Err);

Clang could warn if we remove the std::move...

In D81695#2109038, @tskeith wrote:

In D81695#2108974, @DavidTruby wrote:

There is another example here of our BuildBots failing for very spurious warnings due to -Werror:
http://lab.llvm.org:8014/builders/flang-aarch64-ubuntu-clang/builds/229/steps/build-unified-tree/logs/stdio

The warning is not spurious, so this example shows -Werror in the best possible light:

Absolutely: this is why we should have bots with Werror enabled (but not with gcc-9 for example)
(This is separate to the default setting though)

[DavidTruby]

Can we move ahead with this change before the llvm 11 branch?

[DavidTruby]

@isuruf I believe per a discussion on the community call yesterday that we have consensus for this change now, so do you want to go ahead and commit it? Thanks!

flang/CMakeLists.txt

message(FATAL_ERROR "In-source builds are not allowed. \
           This process created the file `CMakeCache.txt' and the directory\
           `CMakeFiles'. Please delete them.")
 endif()
 
 # Add Flang-centric modules to cmake path.
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules")
 include(AddFlang)
 
-if (MSVC)
-  set(_FLANG_ENABLE_WERROR_DEFAULT OFF)
-else ()
-  set(_FLANG_ENABLE_WERROR_DEFAULT ON)
-endif()
-option(FLANG_ENABLE_WERROR "Fail and stop building flang if a warning is triggered."
-       "${_FLANG_ENABLE_WERROR_DEFAULT}")
+option(FLANG_ENABLE_WERROR "Fail and stop building flang if a warning is triggered." OFF)
 
 # Check for a standalone build and configure as appropriate from
 # there.
 if (CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   message("Building Flang as a standalone project.")
   project(Flang)
   set(FLANG_STANDALONE_BUILD ON)
 
if (FLANG_ENABLE_WERROR)
   # Keep this up-to-date with that file
   if( MSVC )
     append("/WX" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
   endif()
   if ( LLVM_COMPILER_IS_GCC_COMPATIBLE )
     append("-Werror" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
     append("-Wno-error" CMAKE_REQUIRED_FLAGS)
   endif( LLVM_COMPILER_IS_GCC_COMPATIBLE )
-  if (NOT LLVM_ENABLE_WERROR)
-      message(WARNING "FLANG_ENABLE_WERROR setting is different from LLVM_ENABLE_WERROR.")
-  endif()
 endif()
 
 # Builtin check_cxx_compiler_flag doesn't seem to work correctly
 macro(check_compiler_flag flag resultVar)
   unset(${resultVar} CACHE)
   check_cxx_compiler_flag("${flag}" ${resultVar})
 endmacro()