[PATCH] D84292: Only enable -Wsuggest-override if it doesn't suggest adding override to functions that are already final

[Logan Smith via Phabricator via llvm-commits]

logan-5 created this revision.
logan-5 added a reviewer: aaron.ballman.
Herald added subscribers: llvm-commits, mgorny.
Herald added a project: LLVM.

A previous patch added -Wsuggest-override using a simple add_flag_if_supported(). This causes lots of warnings in LLVM when building with older GCC versions (< 9) which suggest adding `override` to functions that are only marked `final`. The current flags in both GCC >=9 and Clang accept plain `final` as equivalent to `override final`.

This patch adds logic to detect versions of -Wsuggest-override that warn on `void foo() final` and disables them to avoid warning spam in builds using older GCC's. This has the added minor benefit of getting rid of the useless C_SUPPORTS_SUGGEST_OVERRIDE_FLAG CMake cache variable which was set by add_flag_if_supported().


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D84292

Files:
  llvm/cmake/modules/HandleLLVMOptions.cmake


Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===================================================================
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -672,8 +672,20 @@
   # 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)
+  # 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 it is disabled for GCC < 9).
+  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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D84292.279701.patch
Type: text/x-patch
Size: 1519 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200722/64585fc8/attachment.bin>