From c2132beee164ae9a36c5bb05fbdec1575333f521 Mon Sep 17 00:00:00 2001 From: Logan Smith Date: Wed, 22 Jul 2020 17:44:52 -0700 Subject: [PATCH] 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. --- cmake/modules/HandleLLVMOptions.cmake | 15 +++++++++++++++ lib/Testing/Support/CMakeLists.txt | 4 ++++ unittests/CMakeLists.txt | 4 ++++ utils/benchmark/CMakeLists.txt | 4 ++++ utils/unittest/CMakeLists.txt | 3 +++ 5 files changed, 30 insertions(+) diff --git a/cmake/modules/HandleLLVMOptions.cmake b/cmake/modules/HandleLLVMOptions.cmake index 2e249593e12..62dd0ef79cf 100644 --- a/cmake/modules/HandleLLVMOptions.cmake +++ b/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/lib/Testing/Support/CMakeLists.txt b/lib/Testing/Support/CMakeLists.txt index fe460aeefc9..595221a105c 100644 --- a/lib/Testing/Support/CMakeLists.txt +++ b/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/unittests/CMakeLists.txt b/unittests/CMakeLists.txt index d7dbaeaa32f..e90f07448c1 100644 --- a/unittests/CMakeLists.txt +++ b/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/utils/benchmark/CMakeLists.txt b/utils/benchmark/CMakeLists.txt index 542c70ca494..3a80b906d5c 100644 --- a/utils/benchmark/CMakeLists.txt +++ b/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/utils/unittest/CMakeLists.txt b/utils/unittest/CMakeLists.txt index 5aad048f2c3..36761a60d9f 100644 --- a/utils/unittest/CMakeLists.txt +++ b/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 )