[PATCH] D84126: Enable -Wsuggest-override in the LLVM build

[Logan Smith via Phabricator via llvm-commits]

logan-5 created this revision.
logan-5 added reviewers: dblaikie, rnk, aaron.ballman, ldionne.
Herald added subscribers: llvm-commits, libcxx-commits, lldb-commits, lebedev.ri, dexonsmith, mgorny.
Herald added a reviewer: lebedev.ri.
Herald added projects: LLDB, libc++, libc++abi, LLVM.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.

This patch adds Clang's new (and GCC's old) `-Wsuggest-override` to the warning flags for the LLVM build. The warning is a stronger form of `-Winconsistent-missing-override` which warns _everywhere_ that `override` is missing, not just in places where it's inconsistent within a class.

Some directories in the monorepo need the warning disabled for compatibility's, or sanity's, sake; in particular, libcxx/libcxxabi (I've added @ldionne as a reviewer to confirm this), and any code implementing or interoperating with googletest, googlemock, or google benchmark (which do not themselves use `override`). This patch adds `-Wno-suggest-override` to the relevant CMakeLists.txt's to accomplish this.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84126

Files:
  libcxx/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  lldb/unittests/CMakeLists.txt
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/utils/benchmark/CMakeLists.txt
  llvm/utils/unittest/CMakeLists.txt


Index: llvm/utils/unittest/CMakeLists.txt
===================================================================
--- llvm/utils/unittest/CMakeLists.txt
+++ llvm/utils/unittest/CMakeLists.txt
@@ -43,6 +43,9 @@
 if(CXX_SUPPORTS_COVERED_SWITCH_DEFAULT_FLAG)
   add_definitions("-Wno-covered-switch-default")
 endif()
+if(CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_definitions("-Wno-suggest-override")
+endif()
 
 set(LLVM_REQUIRES_RTTI 1)
 add_definitions( -DGTEST_HAS_RTTI=0 )
Index: llvm/utils/benchmark/CMakeLists.txt
===================================================================
--- llvm/utils/benchmark/CMakeLists.txt
+++ llvm/utils/benchmark/CMakeLists.txt
@@ -156,6 +156,10 @@
     add_cxx_compiler_flag(-fno-exceptions)
   endif()
 
+  if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+    add_cxx_compiler_flag(-Wno-suggest-override)
+  endif()
+
   if (HAVE_CXX_FLAG_FSTRICT_ALIASING)
     if (NOT CMAKE_CXX_COMPILER_ID STREQUAL "Intel") #ICC17u2: Many false positives for Wstrict-aliasing
       add_cxx_compiler_flag(-Wstrict-aliasing)
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===================================================================
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -672,6 +672,9 @@
   # Enable -Wdelete-non-virtual-dtor if available.
   add_flag_if_supported("-Wdelete-non-virtual-dtor" DELETE_NON_VIRTUAL_DTOR_FLAG)
 
+  # Enable -Wsuggest-override if available.
+  add_flag_if_supported("-Wsuggest-override" SUGGEST_OVERRIDE_FLAG)
+
   # Check if -Wcomment is OK with an // comment ending with '\' if the next
   # line is also a // comment.
   set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
Index: lldb/unittests/CMakeLists.txt
===================================================================
--- lldb/unittests/CMakeLists.txt
+++ lldb/unittests/CMakeLists.txt
@@ -5,6 +5,10 @@
 include_directories(${LLDB_SOURCE_ROOT})
 include_directories(${LLDB_PROJECT_ROOT}/unittests)
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_definitions("-Wno-suggest-override")
+endif()
+
 set(LLDB_GTEST_COMMON_INCLUDE ${CMAKE_CURRENT_SOURCE_DIR}/gtest_common.h)
 if (MSVC)
   list(APPEND LLVM_COMPILE_FLAGS /FI ${LLDB_GTEST_COMMON_INCLUDE})
Index: libcxxabi/CMakeLists.txt
===================================================================
--- libcxxabi/CMakeLists.txt
+++ libcxxabi/CMakeLists.txt
@@ -283,6 +283,8 @@
 add_compile_flags_if_supported(-Wwrite-strings)
 add_compile_flags_if_supported(-Wundef)
 
+add_compile_flags_if_supported(-Wno-suggest-override)
+
 if (LIBCXXABI_ENABLE_WERROR)
   add_compile_flags_if_supported(-Werror)
   add_compile_flags_if_supported(-WX)
Index: libcxx/CMakeLists.txt
===================================================================
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -578,6 +578,7 @@
     target_add_compile_flags_if_supported(${target} PRIVATE
       -Wno-user-defined-literals
       -Wno-covered-switch-default
+      -Wno-suggest-override
       -Wno-ignored-attributes # FIXME: Caused by _LIBCPP_NODEBUG_TYPE not being supported on older clangs
     )
     if (LIBCXX_TARGETING_CLANG_CL)
@@ -602,7 +603,8 @@
     target_add_compile_flags_if_supported(${target} PRIVATE
       -Wno-literal-suffix
       -Wno-c++14-compat
-      -Wno-noexcept-type)
+      -Wno-noexcept-type
+      -Wno-suggest-override)
   endif()
   if (LIBCXX_ENABLE_WERROR)
     target_add_compile_flags_if_supported(${target} PRIVATE -Werror)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84126.279096.patch
Type: text/x-patch
Size: 3470 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200719/578faf58/attachment-0001.bin>