cmake: Fix checking compiler flags like -Wno-some-warning #1332

1. 
   hebasto commented at 9:47 am on May 30, 2023: member

   Some compilers (GCC) produce no diagnostic for -Wno-some-warning unless other diagnostics are being produced:

    0$ git diff
    1diff --git a/CMakeLists.txt b/CMakeLists.txt
    2index 240557f..f976824 100644
    3--- a/CMakeLists.txt
    4+++ b/CMakeLists.txt
    5@@ -220,6 +220,7 @@ if(MSVC)
    6   add_compile_definitions(_CRT_SECURE_NO_WARNINGS)
    7 else()
    8   # Keep the following commands ordered lexicographically.
    9+  try_append_c_flags(-Wno-some-warning)
   10   try_append_c_flags(-pedantic)
   11   try_append_c_flags(-Wall) # GCC >= 2.95 and probably many other compilers.
   12   try_append_c_flags(-Wcast-align) # GCC >= 2.95.
   13$ cmake -B build -DCMAKE_C_COMPILER=gcc 2>&1 | grep -i unknown
   14-- Performing Test C_SUPPORTS__WNO_SOME_WARNING
   15-- Performing Test C_SUPPORTS__WNO_SOME_WARNING - Success
   16Compile options ....................... -Wno-some-warning -pedantic -Wall -Wcast-align -Wcast-align=strict -Wextra -Wnested-externs -Wno-long-long -Wno-overlength-strings -Wno-unused-function -Wshadow -Wstrict-prototypes -Wundef
   

   This PR fixes compiler flag check in CMake-based build system.

   Let me know, if the same fix is desirable in Autotools-based build system.

2. real-or-random added the label build on May 30, 2023
3. 
   fanquake commented at 4:40 pm on July 1, 2024: member

   Is this currently an issue for anything? If the check is broken, then it should be fixed before we start using it.
4. 
   real-or-random commented at 4:48 pm on July 1, 2024: contributor

   Is this currently an issue for anything? If the check is broken, then it should be fixed before we start using it.

   I think noone has run into this so far, but yeah, we should fix it.

   Concept ACK

5. in cmake/TryAppendCFlags.cmake:16 in 1e97094795 outdated

    9@@ -10,7 +10,13 @@ function(secp256k1_check_c_flags_internal flags output)
   10 
   11   # This avoids running a linker.
   12   set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY)
   13-  check_c_compiler_flag("${flags}" ${result})
   14+
   15+  # Some compilers (GCC) produce no diagnostic for -Wno-unknown-warning
   16+  # unless other diagnostics are being produced. Therefore, test the
   17+  # -Wsome-warning case instead of the -Wno-some-warning one.
   

   ---

   ---

   

   real-or-random commented at 4:52 pm on July 1, 2024:

   0  # Some compilers (GCC) produce no diagnostic for -Wno-some-warning,
   1  # if "some-warning" is unknown to the compiler and no other diagnostics are being produced.
   2  #  Therefore, test the
   3  # -Wsome-warning case instead of the -Wno-some-warning one.
   

   Modulo paragraph formatting.

   This confused me a lot because I thought -Wno-unknown-warning is a real flag that says “Don’t warn on unknown warnings flags”. (Clang has a real flag -Wno-unknown-warning-option for this!)

   ---

   

   hebasto commented at 10:45 pm on July 1, 2024:

   Thanks! Reworked.
6. 
   hebasto commented at 10:37 pm on July 1, 2024: member

   Is this currently an issue for anything?

   The newest current option–-Wno-overlength-strings–is supported for GCC >=4.2. Only someone using an older compiler would be affected.

   If the check is broken, then it should be fixed before we start using it.

   The PR description mentions two checks, one per build system. The check in the Autotools-based build system: https://github.com/bitcoin-core/secp256k1/blob/a5269373fa13ff845f654d81b90629dd78495641/build-aux/m4/bitcoin_secp.m4#L64-L78 is broken.

   The check in the CMake-based build system mirrored the behaviour of its Autotools’ counterpart.

   Therefore, both checks need to be fixed.

7. cmake: Fix checking compiler flags like `-Wno-some-warning` bad556ae29
8. hebasto force-pushed on Jul 1, 2024
9. hebasto marked this as ready for review on Jul 1, 2024

Contributors
hebasto fanquake real-or-random

Labels
build