[cmake] Disable GCC 9's -Wredundant-move

[labath]

Summary

This new warning (enabled by -Wextra) fires when a std::move is
redundant, as the default compiler behavior would be to select a move
operation anyway (e.g., when returning a local variable). Unlike
-Wpessimizing-move, it has no performance impact -- it just adds noise.

Currently llvm has about 1500 of these warnings. Unfortunately, the
suggested fix -- removing std::move -- does not work because of some
older compilers we still support. Specifically clang<=3.8 will not use a
move operation if an implicit conversion is needed (Core issue 1579). In
code like "A f(ConvertibleToA a) { return a; }" it will prefer a copy,
or fail to compile if a copy is not possible.

This patch disables that warning to get a meaningful signal out of a GCC
9 build.

[xbolva00]

Looks like Clang has -Wredundant-move too (part of -Wall). Is it less noisy? I dont see such warnings when boostraping clang. So maybe worth to leave it on for Clang?

[labath]

Interesting. I didn't realize that. I'll take a closer look at that and get back here...

[labath]

Leave the warning on for clang (thanks David).

[labath]

After some archaeology, this is what I've learned. Clang used to (a very long time ago) have -Wredundant-move behavior which mostly matched what the current gcc does. Then, in rL243594 (2015), it's scope was reduced to only cover the simple cases (no type conversion involved). It seems the reason for that was that clang at that time did not implement DR1579, and so the thing which the warning suggested to do was actually invalid. Some time later in 2016, DR1579 was implemented (rL274291). However, the warning remained unchanged.

Anyway, it seems that we can leave the warning enabled for clang, at least until it doesn't become more aggressive.

+@rtrieu (author of rL243594) to see whether it's worth re-enabling that warning now.

[xbolva00]

Ok, thanks for more info.

I think it is fine to disable this warning for gcc and clang too.

[aaron.ballman]

This seems reasonable to me.

llvm/cmake/modules/HandleLLVMOptions.cmake

if (LLVM_ENABLE_WARNINGS AND (LLVM_COMPILER_IS_GCC_COMPATIBLE OR CLANG_CL))
   append_if(USE_NO_UNINITIALIZED "-Wno-uninitialized" CMAKE_CXX_FLAGS)
   append_if(USE_NO_MAYBE_UNINITIALIZED "-Wno-maybe-uninitialized" CMAKE_CXX_FLAGS)
 
   # Disable -Wclass-memaccess, a C++-only warning from GCC 8 that fires on
   # LLVM's ADT classes.
   check_cxx_compiler_flag("-Wclass-memaccess" CXX_SUPPORTS_CLASS_MEMACCESS_FLAG)
   append_if(CXX_SUPPORTS_CLASS_MEMACCESS_FLAG "-Wno-class-memaccess" CMAKE_CXX_FLAGS)
 
+  # Disable -Wredundant-move on GCC>=9. GCC wants to remove std::move in code
+  # like "A foo(ConvertibleToA a) { return std::move(a); }", but this code does
+  # not compile (or uses the copy constructor instead) on clang<=3.8. Clang also
+  # has a -Wredundant-move, but it only fires when the types match exactly, so
+  # we can keep it here.
+  if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+    check_cxx_compiler_flag("-Wredundant-move" CXX_SUPPORTS_REDUNDANT_MOVE_FLAG)
+    append_if(CXX_SUPPORTS_REDUNDANT_MOVE_FLAG "-Wno-redundant-move" CMAKE_CXX_FLAGS)
+  endif()
+
   # The LLVM libraries have no stable C++ API, so -Wnoexcept-type is not useful.
   check_cxx_compiler_flag("-Wnoexcept-type" CXX_SUPPORTS_NOEXCEPT_TYPE_FLAG)
   append_if(CXX_SUPPORTS_NOEXCEPT_TYPE_FLAG "-Wno-noexcept-type" CMAKE_CXX_FLAGS)
 
   # Check if -Wnon-virtual-dtor warns even though the class is marked final.
   # If it does, don't add it. So it won't be added on clang 3.4 and older.
   # This also catches cases when -Wnon-virtual-dtor isn't supported by
   # the compiler at all.  This flag is not activated for gcc since it will