[lld] 77e0e9e - Reapply "Try enabling -Wsuggest-override again, using add_compile_options instead of add_compile_definitions for disabling it in unittests/ directories."

Logan Smith via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 22 17:50:36 PDT 2020 

Author: Logan Smith
Date: 2020-07-22T17:50:19-07:00
New Revision: 77e0e9e17daf0865620abcd41f692ab0642367c4

URL: https://github.com/llvm/llvm-project/commit/77e0e9e17daf0865620abcd41f692ab0642367c4
DIFF: https://github.com/llvm/llvm-project/commit/77e0e9e17daf0865620abcd41f692ab0642367c4.diff

LOG: Reapply "Try enabling -Wsuggest-override again, using add_compile_options instead of add_compile_definitions for disabling it in unittests/ directories."

add_compile_options is more sensitive to its location in the file than add_definitions--it only takes effect for sources that are added after it. This updated patch ensures that the add_compile_options is done before adding any source files that depend on it.

Using add_definitions caused the flag to be passed to rc.exe on Windows and thus broke Windows builds.

Added: 
    

Modified: 
    clang-tools-extra/clangd/unittests/CMakeLists.txt
    clang-tools-extra/unittests/CMakeLists.txt
    clang/unittests/CMakeLists.txt
    compiler-rt/cmake/Modules/AddCompilerRT.cmake
    compiler-rt/cmake/config-ix.cmake
    flang/unittests/CMakeLists.txt
    libcxx/CMakeLists.txt
    libcxxabi/CMakeLists.txt
    lld/unittests/CMakeLists.txt
    lldb/unittests/CMakeLists.txt
    llvm/cmake/modules/HandleLLVMOptions.cmake
    llvm/lib/Testing/Support/CMakeLists.txt
    llvm/unittests/CMakeLists.txt
    llvm/utils/benchmark/CMakeLists.txt
    llvm/utils/unittest/CMakeLists.txt
    mlir/unittests/CMakeLists.txt
    parallel-libs/acxxel/CMakeLists.txt
    polly/unittests/CMakeLists.txt

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt
index c25e2b7f8103..8ede92c16f7a 100644
--- a/clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -13,6 +13,10 @@ include_directories(
   ${CLANGD_BINARY_DIR}
   )
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_compile_options("-Wno-suggest-override")
+endif()
+
 if(CLANG_BUILT_STANDALONE)
   # LLVMTestingSupport library is needed for clangd tests.
   if (EXISTS ${LLVM_MAIN_SRC_DIR}/lib/Testing/Support

diff  --git a/clang-tools-extra/unittests/CMakeLists.txt b/clang-tools-extra/unittests/CMakeLists.txt
index 086a68e63830..72abe0fa6d0c 100644
--- a/clang-tools-extra/unittests/CMakeLists.txt
+++ b/clang-tools-extra/unittests/CMakeLists.txt
@@ -5,6 +5,10 @@ function(add_extra_unittest test_dirname)
   add_unittest(ExtraToolsUnitTests ${test_dirname} ${ARGN})
 endfunction()
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_compile_options("-Wno-suggest-override")
+endif()
+
 add_subdirectory(clang-apply-replacements)
 add_subdirectory(clang-change-namespace)
 add_subdirectory(clang-doc)

diff  --git a/clang/unittests/CMakeLists.txt b/clang/unittests/CMakeLists.txt
index 4c222e24599f..9a52b9fb0262 100644
--- a/clang/unittests/CMakeLists.txt
+++ b/clang/unittests/CMakeLists.txt
@@ -1,6 +1,10 @@
 add_custom_target(ClangUnitTests)
 set_target_properties(ClangUnitTests PROPERTIES FOLDER "Clang tests")
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_compile_options("-Wno-suggest-override")
+endif()
+
 if(CLANG_BUILT_STANDALONE)
   # LLVMTestingSupport library is needed for some of the unittests.
   if (EXISTS ${LLVM_MAIN_SRC_DIR}/lib/Testing/Support

diff  --git a/compiler-rt/cmake/Modules/AddCompilerRT.cmake b/compiler-rt/cmake/Modules/AddCompilerRT.cmake
index dab55707338a..efb660818270 100644
--- a/compiler-rt/cmake/Modules/AddCompilerRT.cmake
+++ b/compiler-rt/cmake/Modules/AddCompilerRT.cmake
@@ -403,6 +403,7 @@ set(COMPILER_RT_GMOCK_CFLAGS
 
 append_list_if(COMPILER_RT_DEBUG -DSANITIZER_DEBUG=1 COMPILER_RT_UNITTEST_CFLAGS)
 append_list_if(COMPILER_RT_HAS_WCOVERED_SWITCH_DEFAULT_FLAG -Wno-covered-switch-default COMPILER_RT_UNITTEST_CFLAGS)
+append_list_if(COMPILER_RT_HAS_WSUGGEST_OVERRIDE_FLAG -Wno-suggest-override COMPILER_RT_UNITTEST_CFLAGS)
 
 if(MSVC)
   # gtest use a lot of stuff marked as deprecated on Windows.

diff  --git a/compiler-rt/cmake/config-ix.cmake b/compiler-rt/cmake/config-ix.cmake
index f535123351d6..0a27910ed494 100644
--- a/compiler-rt/cmake/config-ix.cmake
+++ b/compiler-rt/cmake/config-ix.cmake
@@ -106,6 +106,7 @@ check_cxx_compiler_flag("-Werror -Wnon-virtual-dtor"   COMPILER_RT_HAS_WNON_VIRT
 check_cxx_compiler_flag("-Werror -Wvariadic-macros"    COMPILER_RT_HAS_WVARIADIC_MACROS_FLAG)
 check_cxx_compiler_flag("-Werror -Wunused-parameter"   COMPILER_RT_HAS_WUNUSED_PARAMETER_FLAG)
 check_cxx_compiler_flag("-Werror -Wcovered-switch-default" COMPILER_RT_HAS_WCOVERED_SWITCH_DEFAULT_FLAG)
+check_cxx_compiler_flag("-Werror -Wsuggest-override"   COMPILER_RT_HAS_WSUGGEST_OVERRIDE_FLAG)
 check_cxx_compiler_flag(-Wno-pedantic COMPILER_RT_HAS_WNO_PEDANTIC)
 
 check_cxx_compiler_flag(/W4 COMPILER_RT_HAS_W4_FLAG)

diff  --git a/flang/unittests/CMakeLists.txt b/flang/unittests/CMakeLists.txt
index d53d155f2f2b..21da59f3afcb 100644
--- a/flang/unittests/CMakeLists.txt
+++ b/flang/unittests/CMakeLists.txt
@@ -5,6 +5,10 @@ function(add_flang_unittest test_dirname)
   add_unittest(FlangUnitTests ${test_dirname} ${ARGN})
 endfunction()
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_compile_options("-Wno-suggest-override")
+endif()
+
 add_subdirectory(Optimizer)
 add_subdirectory(Decimal)
 add_subdirectory(Evaluate)

diff  --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index bf2ae67ecd5c..5fa3b66c99a7 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -578,6 +578,7 @@ function(cxx_add_warning_flags target)
     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 @@ function(cxx_add_warning_flags target)
     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)

diff  --git a/libcxxabi/CMakeLists.txt b/libcxxabi/CMakeLists.txt
index 534ef7e8c847..96a1c625222a 100644
--- a/libcxxabi/CMakeLists.txt
+++ b/libcxxabi/CMakeLists.txt
@@ -283,6 +283,8 @@ add_compile_flags_if_supported(-Wunused-variable)
 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)

diff  --git a/lld/unittests/CMakeLists.txt b/lld/unittests/CMakeLists.txt
index 84d35d43f4e8..88cb85a08401 100644
--- a/lld/unittests/CMakeLists.txt
+++ b/lld/unittests/CMakeLists.txt
@@ -12,5 +12,9 @@ function(add_lld_unittest test_dirname)
   target_link_libraries(${test_dirname} ${LLVM_COMMON_LIBS})
 endfunction()
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_compile_options("-Wno-suggest-override")
+endif()
+
 add_subdirectory(DriverTests)
 add_subdirectory(MachOTests)

diff  --git a/lldb/unittests/CMakeLists.txt b/lldb/unittests/CMakeLists.txt
index 6422b726ca69..37a5f972cdec 100644
--- a/lldb/unittests/CMakeLists.txt
+++ b/lldb/unittests/CMakeLists.txt
@@ -5,6 +5,10 @@ add_dependencies(lldb-test-deps LLDBUnitTests)
 include_directories(${LLDB_SOURCE_ROOT})
 include_directories(${LLDB_PROJECT_ROOT}/unittests)
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_compile_options("-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})

diff  --git a/llvm/cmake/modules/HandleLLVMOptions.cmake b/llvm/cmake/modules/HandleLLVMOptions.cmake
index 2e249593e12f..62dd0ef79cf4 100644
--- a/llvm/cmake/modules/HandleLLVMOptions.cmake
+++ b/llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -672,6 +672,21 @@ if (LLVM_ENABLE_WARNINGS AND (LLVM_COMPILER_IS_GCC_COMPATIBLE OR CLANG_CL))
   # Enable -Wdelete-non-virtual-dtor if available.
   add_flag_if_supported("-Wdelete-non-virtual-dtor" DELETE_NON_VIRTUAL_DTOR_FLAG)
 
+  # Enable -Wsuggest-override if it's available, and only if it doesn't
+  # suggest adding 'override' to functions that are already marked 'final'
+  # (which means it is disabled for GCC < 9.2).
+  check_cxx_compiler_flag("-Wsuggest-override" CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+    set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
+    set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -Werror=suggest-override")
+    CHECK_CXX_SOURCE_COMPILES("class base {public: virtual void anchor();};
+                               class derived : base {public: void anchor() final;};
+                               int main() { return 0; }"
+                              CXX_WSUGGEST_OVERRIDE_ALLOWS_ONLY_FINAL)
+    set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})
+    append_if(CXX_WSUGGEST_OVERRIDE_ALLOWS_ONLY_FINAL "-Wsuggest-override" CMAKE_CXX_FLAGS)
+  endif()
+
   # 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})

diff  --git a/llvm/lib/Testing/Support/CMakeLists.txt b/llvm/lib/Testing/Support/CMakeLists.txt
index fe460aeefc91..595221a105cd 100644
--- a/llvm/lib/Testing/Support/CMakeLists.txt
+++ b/llvm/lib/Testing/Support/CMakeLists.txt
@@ -1,6 +1,10 @@
 add_definitions(-DGTEST_LANG_CXX11=1)
 add_definitions(-DGTEST_HAS_TR1_TUPLE=0)
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_compile_options("-Wno-suggest-override")
+endif()
+
 add_llvm_library(LLVMTestingSupport
   Annotations.cpp
   Error.cpp

diff  --git a/llvm/unittests/CMakeLists.txt b/llvm/unittests/CMakeLists.txt
index d7dbaeaa32fe..e90f07448c10 100644
--- a/llvm/unittests/CMakeLists.txt
+++ b/llvm/unittests/CMakeLists.txt
@@ -14,6 +14,10 @@ function(add_llvm_target_unittest test_dir_name)
   add_llvm_unittest(${test_dir_name} DISABLE_LLVM_LINK_LLVM_DYLIB ${ARGN})
 endfunction()
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_compile_options("-Wno-suggest-override")
+endif()
+
 add_subdirectory(ADT)
 add_subdirectory(Analysis)
 add_subdirectory(AsmParser)

diff  --git a/llvm/utils/benchmark/CMakeLists.txt b/llvm/utils/benchmark/CMakeLists.txt
index 542c70ca4948..3a80b906d5cc 100644
--- a/llvm/utils/benchmark/CMakeLists.txt
+++ b/llvm/utils/benchmark/CMakeLists.txt
@@ -156,6 +156,10 @@ else()
     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)

diff  --git a/llvm/utils/unittest/CMakeLists.txt b/llvm/utils/unittest/CMakeLists.txt
index 5aad048f2c35..36761a60d9f7 100644
--- a/llvm/utils/unittest/CMakeLists.txt
+++ b/llvm/utils/unittest/CMakeLists.txt
@@ -43,6 +43,9 @@ endif()
 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 )

diff  --git a/mlir/unittests/CMakeLists.txt b/mlir/unittests/CMakeLists.txt
index 851092c5b56a..1dc07413a885 100644
--- a/mlir/unittests/CMakeLists.txt
+++ b/mlir/unittests/CMakeLists.txt
@@ -5,6 +5,10 @@ function(add_mlir_unittest test_dirname)
   add_unittest(MLIRUnitTests ${test_dirname} ${ARGN})
 endfunction()
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_compile_options("-Wno-suggest-override")
+endif()
+
 add_subdirectory(Analysis)
 add_subdirectory(Dialect)
 add_subdirectory(IR)

diff  --git a/parallel-libs/acxxel/CMakeLists.txt b/parallel-libs/acxxel/CMakeLists.txt
index 7e71446107a5..6391c731903b 100644
--- a/parallel-libs/acxxel/CMakeLists.txt
+++ b/parallel-libs/acxxel/CMakeLists.txt
@@ -36,6 +36,9 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
 # Add warning flags.
 set(CMAKE_CXX_FLAGS  "${CMAKE_CXX_FLAGS} -Wall -Wextra")
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+    add_compile_options("-Wno-suggest-override")
+endif()
 
 add_library(
     acxxel

diff  --git a/polly/unittests/CMakeLists.txt b/polly/unittests/CMakeLists.txt
index fac70383de94..1a6881fde3fd 100644
--- a/polly/unittests/CMakeLists.txt
+++ b/polly/unittests/CMakeLists.txt
@@ -19,6 +19,10 @@ function(add_polly_unittest test_name)
   target_link_libraries(${test_name} PRIVATE Polly)
 endfunction()
 
+if (CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_compile_options("-Wno-suggest-override")
+endif()
+
 add_subdirectory(Isl)
 add_subdirectory(Flatten)
 add_subdirectory(DeLICM)

More information about the llvm-commits mailing list