Merge #84: cmake: Add compiler diagnostic flags

695a5ca fixup! ci: Test CMake edge cases (Hennadii Stepanov) f67e9a7 test: Fix MSVC warning C4101 "unreferenced local variable" (Hennadii Stepanov) 4d9c08f FIXUP: Introduce core_base_interface (Hennadii Stepanov) 172b7f5 cmake: Add compiler diagnostic flags (Hennadii Stepanov) Pull request description: Sorry for not presenting the third commit as fixup ones ready to be auto-squashed. ACKs for top commit: vasild: ACK 695a5ca Tree-SHA512: aa9ea41701c62bd4f282c9eff09b6306d2d8d527c99e05f721e72d0917d08561e14cccaffb69477144e3cd4abca06a767cbe89a17721725a5280d3d0af98e9c1
hebasto · Mar 11, 2024 · d01f59f · d01f59f
2 parents 3521e2f + 695a5ca
commit d01f59f
Show file tree

Hide file tree

Showing 10 changed files with 104 additions and 50 deletions.
diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml
@@ -272,6 +272,8 @@ jobs:
             c_compiler: 'clang-14 -m32'
             cxx_compiler: 'clang++-14 -m32'
             depends_options: ''
+            # For -Wno-error=unreachable-code, please refer to https://github.com/bitcoin/bitcoin/issues/29334
+            configure_env: 'env CXXFLAGS="-Wno-error=unreachable-code"'
             configure_options: '-DWERROR=ON'
           - name: 'MinGW-w64'
             triplet: 'x86_64-w64-mingw32'
@@ -343,7 +345,7 @@ jobs:
 
       - name: Generate build system
         run: |
-          cmake -B build --toolchain depends/${{ matrix.host.triplet }}/share/toolchain.cmake ${{ matrix.host.configure_options }}
+          ${{ matrix.host.configure_env }} cmake -B build --toolchain depends/${{ matrix.host.triplet }}/share/toolchain.cmake ${{ matrix.host.configure_options }}
 
       - name: Build
         run: |
@@ -436,7 +438,7 @@ jobs:
 
       - name: Generate build system
         run: |
-          cmake -B build --preset ${{ matrix.conf.preset }}
+          cmake -B build --preset ${{ matrix.conf.preset }} -DWERROR=ON
 
       - name: Save vcpkg binary cache
         uses: actions/cache/save@v4
@@ -503,6 +505,8 @@ jobs:
           - name: 'Xcode 14.3'
             id: 'xcode-14.3'
             path: '/Applications/Xcode_14.3.app'
+            # For -Wno-error=unreachable-code, please refer to https://github.com/bitcoin/bitcoin/issues/29334
+            configure_env: 'env CXXFLAGS="-Wno-error=unreachable-code"'
           - name: 'Xcode 15.2'
             id: 'xcode-15.2'
             path: '/Applications/Xcode_15.2.app'
@@ -538,7 +542,7 @@ jobs:
 
       - name: Generate build system
         run: |
-          cmake -B build
+          ${{ matrix.xcode.configure_env }} cmake -B build -DWERROR=ON
 
       - name: Build
         run: |

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -107,6 +107,12 @@ unset(check_pie_output)
 # The core_interface library aims to encapsulate common build flags.
 # It is intended to be a usage requirement for all other targets.
 add_library(core_interface INTERFACE)
+# The core_base_interface library is a subset of the core_interface
+# designated to be used with subtrees. In particular, it does not
+# include the warn_interface as subtree's warnings are not fixable
+# in our tree.
+add_library(core_base_interface INTERFACE)
+target_link_libraries(core_interface INTERFACE core_base_interface)
 
 if(FUZZ)
   message(WARNING "FUZZ=ON will disable all other targets and force BUILD_FUZZ_BINARY=ON.")
@@ -150,7 +156,7 @@ if(WIN32)
   - A run-time library must be specified explicitly using _MT definition.
   ]=]
 
-  target_compile_definitions(core_interface INTERFACE
+  target_compile_definitions(core_base_interface INTERFACE
     _WIN32_WINNT=0x0601
     _WIN32_IE=0x0501
     WIN32_LEAN_AND_MEAN
@@ -166,7 +172,7 @@ if(WIN32)
     set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>${msvc_library_linkage}")
     unset(msvc_library_linkage)
 
-    target_compile_definitions(core_interface INTERFACE
+    target_compile_definitions(core_base_interface INTERFACE
       _UNICODE;UNICODE
     )
     target_compile_options(core_interface INTERFACE
@@ -176,66 +182,55 @@ if(WIN32)
     # Improve parallelism in MSBuild.
     # See: https://devblogs.microsoft.com/cppblog/improved-parallelism-in-msbuild/.
     list(APPEND CMAKE_VS_GLOBALS "UseMultiToolTask=true")
-
-    try_append_cxx_flags("/W3" TARGET core_interface SKIP_LINK)
-    try_append_cxx_flags("/wd4018" TARGET core_interface SKIP_LINK)
-    try_append_cxx_flags("/wd4244" TARGET core_interface SKIP_LINK)
-    try_append_cxx_flags("/wd4267" TARGET core_interface SKIP_LINK)
-    try_append_cxx_flags("/wd4715" TARGET core_interface SKIP_LINK)
-    try_append_cxx_flags("/wd4805" TARGET core_interface SKIP_LINK)
-    target_compile_definitions(core_interface INTERFACE
-      _CRT_SECURE_NO_WARNINGS
-      _SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING
-    )
   endif()
 
   if(MINGW)
-    target_compile_definitions(core_interface INTERFACE
+    target_compile_definitions(core_base_interface INTERFACE
       WIN32
       _WINDOWS
       _MT
     )
     # Avoid the use of aligned vector instructions when building for Windows.
     # See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54412.
-    try_append_cxx_flags("-Wa,-muse-unaligned-vector-move" TARGET core_interface SKIP_LINK)
-    try_append_linker_flag("-static" TARGET core_interface)
+    try_append_cxx_flags("-Wa,-muse-unaligned-vector-move" TARGET core_base_interface SKIP_LINK)
+    try_append_linker_flag("-static" TARGET core_base_interface)
     # We require Windows 7 (NT 6.1) or later.
-    try_append_linker_flag("-Wl,--major-subsystem-version,6" TARGET core_interface)
-    try_append_linker_flag("-Wl,--minor-subsystem-version,1" TARGET core_interface)
+    try_append_linker_flag("-Wl,--major-subsystem-version,6" TARGET core_base_interface)
+    try_append_linker_flag("-Wl,--minor-subsystem-version,1" TARGET core_base_interface)
   endif()
 endif()
 
 # Use 64-bit off_t on 32-bit Linux.
 if (CMAKE_SYSTEM_NAME STREQUAL "Linux" AND CMAKE_SIZEOF_VOID_P EQUAL 4)
   # Ensure 64-bit offsets are used for filesystem accesses for 32-bit compilation.
-  target_compile_definitions(core_interface INTERFACE
+  target_compile_definitions(core_base_interface INTERFACE
     _FILE_OFFSET_BITS=64
   )
 endif()
 
 if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
-  target_compile_definitions(core_interface INTERFACE
+  target_compile_definitions(core_base_interface INTERFACE
     MAC_OSX
   )
   # These flags are specific to ld64, and may cause issues with other linkers.
   # For example: GNU ld will interpret -dead_strip as -de and then try and use
   # "ad_strip" as the symbol for the entry point.
-  try_append_linker_flag("-Wl,-dead_strip" TARGET core_interface)
-  try_append_linker_flag("-Wl,-dead_strip_dylibs" TARGET core_interface)
-  try_append_linker_flag("-Wl,-headerpad_max_install_names" TARGET core_interface)
+  try_append_linker_flag("-Wl,-dead_strip" TARGET core_base_interface)
+  try_append_linker_flag("-Wl,-dead_strip_dylibs" TARGET core_base_interface)
+  try_append_linker_flag("-Wl,-headerpad_max_install_names" TARGET core_base_interface)
 endif()
 
 if(CMAKE_CROSSCOMPILING)
-  target_compile_definitions(core_interface INTERFACE ${DEPENDS_COMPILE_DEFINITIONS})
-  target_compile_options(core_interface INTERFACE "$<$<COMPILE_LANGUAGE:C>:${DEPENDS_C_COMPILER_FLAGS}>")
-  target_compile_options(core_interface INTERFACE "$<$<COMPILE_LANGUAGE:CXX>:${DEPENDS_CXX_COMPILER_FLAGS}>")
+  target_compile_definitions(core_base_interface INTERFACE ${DEPENDS_COMPILE_DEFINITIONS})
+  target_compile_options(core_base_interface INTERFACE "$<$<COMPILE_LANGUAGE:C>:${DEPENDS_C_COMPILER_FLAGS}>")
+  target_compile_options(core_base_interface INTERFACE "$<$<COMPILE_LANGUAGE:CXX>:${DEPENDS_CXX_COMPILER_FLAGS}>")
 endif()
 
 include(AddThreadsIfNeeded)
 add_threads_if_needed()
 
 add_library(sanitizing_interface INTERFACE)
-target_link_libraries(core_interface INTERFACE sanitizing_interface)
+target_link_libraries(core_base_interface INTERFACE sanitizing_interface)
 if(SANITIZERS)
   # First check if the compiler accepts flags. If an incompatible pair like
   # -fsanitize=address,thread is used here, this check will fail. This will also
@@ -282,11 +277,65 @@ include(cmake/leveldb.cmake)
 include(cmake/minisketch.cmake)
 include(cmake/secp256k1.cmake)
 
+string(STRIP "${CMAKE_CXX_FLAGS}" cxx_flags)
+string(STRIP "${CMAKE_CXX_FLAGS_INIT}" cxx_flags_init)
+if(cxx_flags STREQUAL cxx_flags_init)
+  # CMAKE_CXX_FLAGS are not overridden.
+  add_library(warn_interface INTERFACE)
+  target_link_libraries(core_interface INTERFACE warn_interface)
+  if(MSVC)
+    try_append_cxx_flags("/W3" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("/wd4018" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("/wd4244" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("/wd4267" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("/wd4715" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("/wd4805" TARGET warn_interface SKIP_LINK)
+    target_compile_definitions(warn_interface INTERFACE
+      _CRT_SECURE_NO_WARNINGS
+      _SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING
+    )
+  else()
+    try_append_cxx_flags("-Wall" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wextra" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wgnu" TARGET warn_interface SKIP_LINK)
+    # Some compilers will ignore -Wformat-security without -Wformat, so just combine the two here.
+    try_append_cxx_flags("-Wformat;-Wformat-security" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wvla" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wshadow-field" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wthread-safety" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wloop-analysis" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wredundant-decls" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wunused-member-function" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wdate-time" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wconditional-uninitialized" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wduplicated-branches" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wduplicated-cond" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wlogical-op" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Woverloaded-virtual" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wsuggest-override" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wimplicit-fallthrough" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wunreachable-code" TARGET warn_interface SKIP_LINK)
+    try_append_cxx_flags("-Wdocumentation" TARGET warn_interface SKIP_LINK)
+
+    # Some compilers (gcc) ignore unknown -Wno-* options, but warn about all
+    # unknown options if any other warning is produced. Test the -Wfoo case, and
+    # set the -Wno-foo case if it works.
+    try_append_cxx_flags("-Wunused-parameter" TARGET warn_interface SKIP_LINK
+      IF_CHECK_PASSED "-Wno-unused-parameter"
+    )
+    try_append_cxx_flags("-Wself-assign" TARGET warn_interface SKIP_LINK
+      IF_CHECK_PASSED "-Wno-self-assign"
+    )
+  endif()
+endif()
+unset(cxx_flags)
+unset(cxx_flags_init)
+
 include(ProcessConfigurations)
 set_default_config(RelWithDebInfo)
 
 # Redefine configuration flags.
-target_compile_definitions(core_interface INTERFACE
+target_compile_definitions(core_base_interface INTERFACE
   $<$<CONFIG:Debug>:DEBUG>
   $<$<CONFIG:Debug>:DEBUG_LOCKORDER>
   $<$<CONFIG:Debug>:DEBUG_LOCKCONTENTION>
@@ -322,17 +371,17 @@ include(cmake/optional_qt.cmake)
 include(cmake/optional.cmake)
 
 # Don't allow extended (non-ASCII) symbols in identifiers. This is easier for code review.
-try_append_cxx_flags("-fno-extended-identifiers" TARGET core_interface)
+try_append_cxx_flags("-fno-extended-identifiers" TARGET core_base_interface)
 
 # Currently all versions of gcc are subject to a class of bugs, see the
 # gccbug_90348 test case (only reproduces on GCC 11 and earlier) and
 # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111843. To work around that, set
 # -fstack-reuse=none for all gcc builds. (Only gcc understands this flag).
-try_append_cxx_flags("-fstack-reuse=none" TARGET core_interface)
+try_append_cxx_flags("-fstack-reuse=none" TARGET core_base_interface)
 
 if(HARDENING)
   add_library(hardening_interface INTERFACE)
-  target_link_libraries(core_interface INTERFACE hardening_interface)
+  target_link_libraries(core_base_interface INTERFACE hardening_interface)
   if(MSVC)
     try_append_linker_flag("/DYNAMICBASE" TARGET hardening_interface)
     try_append_linker_flag("/HIGHENTROPYVA" TARGET hardening_interface)
@@ -375,7 +424,7 @@ endif()
 if(REDUCE_EXPORTS)
   set(CMAKE_CXX_VISIBILITY_PRESET hidden)
   set(CMAKE_VISIBILITY_INLINES_HIDDEN 1)
-  try_append_linker_flag("-Wl,--exclude-libs,ALL" TARGET core_interface)
+  try_append_linker_flag("-Wl,--exclude-libs,ALL" TARGET core_base_interface)
 endif()
 
 if(WERROR)
@@ -384,7 +433,7 @@ if(WERROR)
   else()
     set(werror_flag "-Werror")
   endif()
-  try_append_cxx_flags(${werror_flag} TARGET core_interface RESULT_VAR compiler_supports_werror SKIP_LINK)
+  try_append_cxx_flags(${werror_flag} TARGET core_base_interface SKIP_LINK RESULT_VAR compiler_supports_werror)
   if(NOT compiler_supports_werror)
     message(FATAL_ERROR "WERROR set but ${werror_flag} is not usable.")
   endif()
@@ -410,7 +459,8 @@ setup_split_debug_script()
 add_maintenance_targets()
 add_windows_deploy_target()
 
-get_target_property(definitions core_interface INTERFACE_COMPILE_DEFINITIONS)
+include(GetTargetInterface)
+get_target_interface(definitions core_interface COMPILE_DEFINITIONS)
 separate_by_configs(definitions)
 
 message("\n")
@@ -452,7 +502,6 @@ message("C++ compiler .......................... ${CMAKE_CXX_COMPILER}")
 list(JOIN DEPENDS_CXX_COMPILER_FLAGS " " depends_cxx_flags)
 string(STRIP "${CMAKE_CXX_FLAGS} ${depends_cxx_flags}" combined_cxx_flags)
 message("CXXFLAGS .............................. ${combined_cxx_flags}")
-include(GetTargetInterface)
 get_target_interface(common_compile_options core_interface COMPILE_OPTIONS)
 message("Common compile options ................ ${common_compile_options}")
 get_target_interface(common_link_options core_interface LINK_OPTIONS)

diff --git a/cmake/crc32c.cmake b/cmake/crc32c.cmake
@@ -112,4 +112,4 @@ if(HAVE_ARM64_CRC32C)
   )
 endif()
 
-target_link_libraries(crc32c PRIVATE core_interface)
+target_link_libraries(crc32c PRIVATE core_base_interface)
diff --git a/cmake/leveldb.cmake b/cmake/leveldb.cmake
@@ -95,7 +95,7 @@ else()
 endif()
 
 target_link_libraries(leveldb PRIVATE
-  core_interface
+  core_base_interface
   nowarn_leveldb_interface
   crc32c
 )
diff --git a/cmake/minisketch.cmake b/cmake/minisketch.cmake
@@ -55,7 +55,7 @@ if(HAVE_CLMUL)
   target_compile_options(minisketch_clmul PRIVATE ${CLMUL_CXXFLAGS})
   target_link_libraries(minisketch_clmul
     PRIVATE
-      core_interface
+      core_base_interface
       minisketch_common
   )
 endif()
@@ -79,7 +79,7 @@ target_include_directories(minisketch
 
 target_link_libraries(minisketch
   PRIVATE
-    core_interface
+    core_base_interface
     minisketch_common
     $<TARGET_NAME_IF_EXISTS:minisketch_clmul>
 )
diff --git a/cmake/module/GetTargetInterface.cmake b/cmake/module/GetTargetInterface.cmake
@@ -8,7 +8,9 @@ include_guard(GLOBAL)
 function(get_target_interface var target property)
   get_target_property(result ${target} INTERFACE_${property})
   if(result)
-    string(GENEX_STRIP "${result}" result)
+    if(NOT ${property} STREQUAL "COMPILE_DEFINITIONS")
+      string(GENEX_STRIP "${result}" result)
+    endif()
     list(JOIN result " " result)
   else()
     set(result)

diff --git a/cmake/secp256k1.cmake b/cmake/secp256k1.cmake
@@ -55,4 +55,4 @@ if(MSVC)
   )
 endif()
 
-target_link_libraries(secp256k1 PRIVATE core_interface)
+target_link_libraries(secp256k1 PRIVATE core_base_interface)
diff --git a/src/qt/CMakeLists.txt b/src/qt/CMakeLists.txt
@@ -102,6 +102,10 @@ target_include_directories(bitcoinqt
   PUBLIC
     $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/src>
 )
+set_property(SOURCE macnotificationhandler.mm
+  # Ignore warnings "'NSUserNotificationCenter' is deprecated: first deprecated in macOS 11.0".
+  APPEND PROPERTY COMPILE_OPTIONS -Wno-deprecated-declarations
+)
 target_link_libraries(bitcoinqt
   PUBLIC
     Qt5::Widgets

diff --git a/src/univalue/test/object.cpp b/src/univalue/test/object.cpp
@@ -20,7 +20,7 @@
         try { \
             (stmt); \
             assert(0 && "No exception caught"); \
-        } catch (excMatch & e) { \
+        } catch (excMatch&) { \
 	} catch (...) { \
 	    assert(0 && "Wrong exception caught"); \
 	} \

diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt
@@ -41,11 +41,6 @@ add_library(bitcoin_util STATIC EXCLUDE_FROM_ALL
   ../sync.cpp
 )
 
-target_compile_definitions(bitcoin_util
-  PRIVATE
-    $<$<CXX_COMPILER_ID:MSVC>:_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING>
-)
-
 target_link_libraries(bitcoin_util
   PRIVATE
     core_interface