Use INTERFACE_COMPILE_OPTIONS to disable -Wsuggest-override for any target that links to gtest

[logan-5]

Summary

This cleans up several CMakeLists.txt's where -Wno-suggest-override was manually specified. These test targets now inherit this flag from the gtest target.

Some unittests CMakeLists.txt's, in particular Flang and LLDB, are not touched by this patch. Flang manually adds the gtest sources itself in some configurations, rather than linking to LLVM's gtest target, so this fix would be insufficient to cover those cases. Similarly, LLDB has subdirectories that manually add the gtest headers to their include path without linking to the gtest target, so those subdirectories still need -Wno-suggest-override to be manually specified to compile without warnings.

[labath]

This looks good to me. Thanks.

Could you elaborate on the lldb issue? I'd like to take a look at that...

[logan-5]

In D84554#2175012, @labath wrote:

Could you elaborate on the lldb issue? I'd like to take a look at that...

It looks like lldb/unittests/TestingSupport/CMakeLists.txt and lldb/unittests/TestingSupport/Symbol/CMakeLists.txt both both manually pull in the googletest/googlemock headers via include_directories. It also seems, although I'm not sure, like they don't themselves link to gtest, since simply removing those include_directories didn't work.

[labath]

In D84554#2176041, @logan-5 wrote:

In D84554#2175012, @labath wrote:

Could you elaborate on the lldb issue? I'd like to take a look at that...

It looks like lldb/unittests/TestingSupport/CMakeLists.txt and lldb/unittests/TestingSupport/Symbol/CMakeLists.txt both both manually pull in the googletest/googlemock headers via include_directories. It also seems, although I'm not sure, like they don't themselves link to gtest, since simply removing those include_directories didn't work.

Thanks. I think I understand the problem now. Those libraries should be linking against gtest, but they don't do that because they could get away with not doing it. And they add include directories manually because they do not get that from linking against gtest. That means we have more things to fix => D84748.

clang-tools-extra/clangd/unittests/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   support
   AllTargetsInfos
   FrontendOpenMP
   )
 
 get_filename_component(CLANGD_SOURCE_DIR
   ${CMAKE_CURRENT_SOURCE_DIR}/../../clangd REALPATH)
 get_filename_component(CLANGD_BINARY_DIR
   ${CMAKE_CURRENT_BINARY_DIR}/../../clangd REALPATH)
 include_directories(
   ${CLANGD_SOURCE_DIR}
   ${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
       AND NOT TARGET LLVMTestingSupport)
     add_subdirectory(${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
       lib/Testing/Support)
   endif()
 endif()

clang-tools-extra/unittests/CMakeLists.txt

 add_custom_target(ExtraToolsUnitTests)
 set_target_properties(ExtraToolsUnitTests PROPERTIES FOLDER "Extra Tools Unit Tests")
 
 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)
 add_subdirectory(clang-include-fixer)
 add_subdirectory(clang-move)
 add_subdirectory(clang-query)
 add_subdirectory(clang-tidy)

clang/unittests/CMakeLists.txt

 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
       AND NOT TARGET LLVMTestingSupport)
     add_subdirectory(${LLVM_MAIN_SRC_DIR}/lib/Testing/Support
       lib/Testing/Support)
   endif()
 endif()

lld/unittests/CMakeLists.txt

 add_custom_target(LLDUnitTests)
 set_target_properties(LLDUnitTests PROPERTIES FOLDER "lld tests")
 
 set(CMAKE_BUILD_WITH_INSTALL_RPATH OFF)
 
 # add_lld_unittest(test_dirname file1.cpp file2.cpp)
 #
 # Will compile the list of files together and link against lld
 # Produces a binary named 'basename(test_dirname)'.
 function(add_lld_unittest test_dirname)
   add_unittest(LLDUnitTests ${test_dirname} ${ARGN})
   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)

llvm/lib/Testing/Support/CMakeLists.txt

 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
   SupportHelpers.cpp
 
   BUILDTREE_ONLY
   
   ADDITIONAL_HEADER_DIRS

llvm/unittests/CMakeLists.txt

 add_custom_target(UnitTests)
 set_target_properties(UnitTests PROPERTIES FOLDER "Tests")
 
 function(add_llvm_unittest test_dirname)
   add_unittest(UnitTests ${test_dirname} ${ARGN})
 endfunction()
 function(add_llvm_unittest_with_input_files test_dirname)
   add_unittest_with_input_files(UnitTests ${test_dirname} ${ARGN})
 endfunction()
 
 # The target unittests may test APIs that aren't exported in libLLVM.so, so
 # we need to always link against the static libraries.
 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)
 add_subdirectory(BinaryFormat)
 add_subdirectory(Bitcode)
 add_subdirectory(Bitstream)
 add_subdirectory(CodeGen)
 add_subdirectory(DebugInfo)

llvm/utils/unittest/CMakeLists.txt

if(SUPPORTS_VARIADIC_MACROS_FLAG)
   add_definitions("-Wno-variadic-macros")
 endif()
 if(SUPPORTS_GNU_ZERO_VARIADIC_MACRO_ARGUMENTS_FLAG)
   add_definitions("-Wno-gnu-zero-variadic-macro-arguments")
 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 )
 
 if (NOT LLVM_ENABLE_THREADS)
   add_definitions( -DGTEST_HAS_PTHREAD=0 )
 endif()
 
add_llvm_library(gtest
 
   LINK_COMPONENTS
   Support # Depends on llvm::raw_ostream
 
   # This is a library meant only for the build tree.
   BUILDTREE_ONLY
 )
 
+# The googletest and googlemock sources don't presently use the 'override' 
+# keyword, which leads to lots of warnings from -Wsuggest-override. Disable 
+# that warning here for any targets that link to gtest.
+if(CXX_SUPPORTS_SUGGEST_OVERRIDE_FLAG)
+  add_definitions("-Wno-suggest-override")
+  set_target_properties(gtest PROPERTIES INTERFACE_COMPILE_OPTIONS "-Wno-suggest-override")
+endif()
+
 add_subdirectory(UnitTestMain)
 
 # When LLVM_LINK_LLVM_DYLIB is enabled, libLLVM.so is added to the interface
 # link libraries for gtest and gtest_main.  This means that any target, like
 # unittests for example, that links against gtest will be forced to link
 # against libLLVM.so.  In some cases we may want to statically unittests if they
 # need access to symbols that are marked private in libLLVM.so.  The only
 # way we can make this work is to remove libLLVM.so from the list of interface

mlir/unittests/CMakeLists.txt

 add_custom_target(MLIRUnitTests)
 set_target_properties(MLIRUnitTests PROPERTIES FOLDER "MLIR Tests")
 
 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)
 add_subdirectory(Pass)
 add_subdirectory(SDBM)
 add_subdirectory(TableGen)

polly/unittests/CMakeLists.txt

else()
     target_link_libraries(${test_name} PRIVATE gtest_main gtest)
     add_dependencies(PollyUnitTests ${test_name})
 
     set_property(TARGET ${test_name} PROPERTY FOLDER "Polly")
   endif()
   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)
 add_subdirectory(ScopPassManager)
 add_subdirectory(ScheduleOptimizer)
 add_subdirectory(Support)