util/check: avoid unused parameter warnings #24729

Add [[maybe_unused]] annotations to avoid warnings from gcc 9.4 and earlier which don't analyse if constexpr properly.

cc @jnewbery ; context https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81676 and #24714 (comment)

Thanks. Fix works for me on gcc 9.4.0.

Can you add a code comment that these annotations are added to work around a bug, referencing https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81676?

http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0212r0.pdf suggests that the warning is not a bug so much as a quality of implementation issue, so not sure it's worth documenting in the code or removing later?

review ACK 0add4dbadbc972933b0c99813a155a4ed4852975

IIRC I tested this yesterday.

ACK 0add4dbadbc972933b0c99813a155a4ed4852975 review and debug build on clang 15

That said, I am starting a debug build with gcc Debian <strike>8.4.0</strike> 9.4.0, will update this review after.

Regarding the C++ attribute maybe_unused (since C++17), had a look at https://en.cppreference.com/w/cpp/language/attributes/maybe_unused and https://stackoverflow.com/a/49320892

the warning is not a bug so much as a quality of implementation issue

It's surely a bug. If a variable is used in a if constexpr branch, then it's used.

Maybe Debian was patched. Master compiled with gcc (Debian 9.4.0-2) 9.4.0 after make distclean builds cleanly for me, couldn't reproduce the warnings.

Options used to compile and link:
  external signer = yes
  multiprocess    = no
  with experimental syscall sandbox support = yes
  with libs       = yes
  with wallet     = yes
    with sqlite   = yes
    with bdb      = yes
  with gui / qt   = yes
    with qr       = yes
  with zmq        = yes
  with test       = yes
  with fuzz binary = yes
  with bench      = yes
  with upnp       = yes
  with natpmp     = yes
  use asm         = yes
  USDT tracing    = yes
  sanitizers      = 
  debug enabled   = yes
  gprof enabled   = no
  werror          = yes
  LTO             = no

  target os       = linux-gnu
  build os        = linux-gnu

  CC              = /usr/bin/ccache gcc
  CFLAGS          = -pthread -g -O2
  CPPFLAGS        =  -DDEBUG -DDEBUG_LOCKORDER -DABORT_ON_FAILED_ASSUME -fmacro-prefix-map=$(abs_top_srcdir)=.  -DDEBUG_LOCKORDER -DHAVE_BUILD_INFO -DPROVIDE_FUZZ_MAIN_FUNCTION
  CXX             = /usr/bin/ccache g++ -std=c++17
  CXXFLAGS        =   -O0 -g3 -ftrapv -fdebug-prefix-map=$(abs_top_srcdir)=.  -fstack-reuse=none -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection  -Wall -Wextra -Wformat -Wformat-security -Wvla -Wredundant-decls -Wdate-time -Wduplicated-branches -Wduplicated-cond -Wlogical-op -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough  -Wno-unused-parameter -Werror   -fno-extended-identifiers
  LDFLAGS         =  -lpthread  -Wl,-z,relro -Wl,-z,now -Wl,-z,separate-code -pie  
  ARFLAGS         = cr

No warnings on master with gcc (Debian 8.4.0-7) 8.4.0 either.

debug enabled = yes

I think you'll need debug disabled, otherwise Assume is treated as Assert

It's surely a bug. If a variable is used in a if constexpr branch, then it's used.

Comments in https://developercommunity2.visualstudio.com/t/constexpr-if-and-unused-parameter/276272 seem to indicate some people expect the warning. In so far as if constexpr is a replacement for #if it seems plausible to mark variables that will be used only in some compile-time configurations in the same way.

I don't think there is any risk in leaving the annotations even if the minimum gcc is bumped to gcc-10. Hopefully the code won't need to be touched further, now that all previously outstanding issues with it have been fixed.

debug enabled = yes

I think you'll need debug disabled, otherwise Assume is treated as Assert

Oh, right.

ACK 0add4dbadbc972933b0c99813a155a4ed4852975

• Verified that the changes in this PR have removed the warning messages while compiling.
• Used configuration:
  • Ubuntu 20.04
  • gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0
  • g++ (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0

Still no warnings (master @ 7ab9fc32d6a88, non-debug gcc 8.4.0-7 build) for me :detective: :smile:

₿ gcc --version
gcc (Debian 8.4.0-7) 8.4.0

₿ make distclean && ./autogen.sh && ./configure --with-incompatible-bdb && make clean && make -j5

.../...

Options used to compile and link:
  external signer = yes
  multiprocess    = no
  with experimental syscall sandbox support = yes
  with libs       = yes
  with wallet     = yes
    with sqlite   = yes
    with bdb      = yes
  with gui / qt   = yes
    with qr       = yes
  with zmq        = yes
  with test       = yes
  with fuzz binary = yes
  with bench      = yes
  with upnp       = yes
  with natpmp     = yes
  use asm         = yes
  USDT tracing    = yes
  sanitizers      = 
  debug enabled   = no
  gprof enabled   = no
  werror          = no
  LTO             = no

  target os       = linux-gnu
  build os        = linux-gnu

  CC              = /usr/bin/ccache gcc
  CFLAGS          = -pthread -g -O2
  CPPFLAGS        =  -fmacro-prefix-map=$(abs_top_srcdir)=.  -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2  -DHAVE_BUILD_INFO -DPROVIDE_FUZZ_MAIN_FUNCTION
  CXX             = /usr/bin/ccache g++ -std=c++17
  CXXFLAGS        =   -fdebug-prefix-map=$(abs_top_srcdir)=.  -fstack-reuse=none -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection  -Wall -Wextra -Wformat -Wformat-security -Wvla -Wredundant-decls -Wdate-time -Wduplicated-branches -Wduplicated-cond -Wlogical-op -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough  -Wno-unused-parameter -Wno-deprecated-copy   -g -O2 -fno-extended-identifiers

gcc (Debian 8.4.0-7) 8.4.0

Not sure which Debian you are using, but buster is the only one shipping gcc-8, which is 8.3: https://packages.debian.org/buster/gcc-8

The steps to reproduce on a fresh install of buster are:

export DEBIAN_FRONTEND=noninteractive && apt update && apt install curl wget htop git vim ccache -y && git clone https://github.com/bitcoin/bitcoin.git ./bitcoin-core && cd bitcoin-core && apt install build-essential libtool autotools-dev automake pkg-config bsdmainutils python3-zmq     libevent-dev libboost-dev  -y   &&  ./autogen.sh && ./configure && make -j $(nproc)

Going to merge this now. I think the discussion whether to add a comment or TODO can be continued and concluded in a potential follow-up. No need to hold back the changes here, which are needed either way.

Thanks (I'm using Debian testing and update-alternatives for managing multiple compiler versions).