Enable `-Wunused` #362

fanquake commented at 10:40 AM on September 9, 2026: member

If merged, this would be used downstream in https://github.com/bitcoin/bitcoin/pull/36167.

DrahtBot commented at 10:40 AM on September 9, 2026: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline and AI policy for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#342 (Allow request cancellation for wrapped C++ methods by xyzconstant)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

fanquake force-pushed on Sep 9, 2026

fanquake force-pushed on Sep 10, 2026

fanquake commented at 1:19 PM on September 10, 2026: member

Not quite sure what is happening here. The job is failing with the following (https://github.com/bitcoin-core/libmultiprocess/actions/runs/34366056349/job/102514949616?pr=362#step:6:253):

Warning: include-what-you-use reported diagnostics:
error: unknown warning option '-Wtrailing-whitespace'; did you mean '-Wunicode-whitespace'? [-Werror,-Wunknown-warning-option]

However -Wtrailing-whitespace already exists in the config on master, so I don't see why the CI would be passing on the master branch, but failing with that issue here. From what I can tell, even though GCC is the compiler being used by CMake (where this flag is supported), include-what-you-use is parsing using Clang, which doesn't support the flag.

fanquake force-pushed on Sep 10, 2026

fanquake commented at 1:30 PM on September 10, 2026: member

Adding -Wno-unused-command-line-argument fixes the unknown warning option error (that makes sense as a side-effect, given the -Wunused addition), however the job is still failing with IWYU issues, that aren't present on master?

maflcko commented at 10:28 AM on September 16, 2026: member

Looks like this also exposes a build race in https://github.com/bitcoin-core/libmultiprocess/actions/runs/34482806488/job/102889597378?pr=362#step:6:384?

DrahtBot added the label Needs rebase on Oct 1, 2026

Sjors commented at 10:03 AM on October 28, 2026: member

#371 was merged, can you rebase? Or are you waiting for something else?

fanquake force-pushed on Oct 28, 2026

refactor: remove unused code 0e6fffa811

refactor: use requires clauses instead of SFINAE enable parameters

Replace the unused `enable` default parameters with C++20 requires
clauses.

fd75dc6696

in include/mp/type-message.h:17 in 750eb521a7

  13 | @@ -14,7 +14,7 @@ namespace mp {
  14 |  //! function, not a template function, but less flexible.
  15 |  template <typename LocalType, typename Value, typename Output>
  16 |  void CustomBuildField(TypeList<LocalType>, Priority<2>, InvokeContext& invoke_context, Value&& value, Output&& output,
  17 | -                      decltype(CustomBuildMessage(invoke_context, value, std::move(output.get())))* enable = nullptr)
  18 | +                      decltype(CustomBuildMessage(invoke_context, value, std::move(output.get())))* = nullptr)

maflcko commented at 10:31 AM on October 28, 2026:

removing the enable here seems like it is harder to read? Either use concepts or the Require port from this project?

fanquake commented at 11:01 AM on October 30, 2026:

Sure, pushed something up.

fanquake force-pushed on Oct 28, 2026

DrahtBot removed the label Needs rebase on Oct 28, 2026

fanquake force-pushed on Oct 28, 2026

ci: add -Wunused to CI configs

Along with -Wunused-const-variable.

ee8e64b87b

in ci/configs/default.bash:3 in 1aa0010826

   0 | @@ -1,5 +1,5 @@
   1 |  CI_DESC="CI job using default libraries and tools, and running IWYU"
   2 |  CI_DIR=build-default
   3 | -export CXXFLAGS="-Werror -Wall -Wextra -Wpedantic -Wundef -Wunused-const-variable -Wno-unused-parameter -Wextra-semi -Wmissing-noreturn -Wtrailing-whitespace"
   4 | +export CXXFLAGS="-Werror -Wall -Wextra -Wpedantic -Wundef -Wunused -Wextra-semi -Wmissing-noreturn -Wtrailing-whitespace"

Sjors commented at 12:46 PM on October 28, 2026:

In 1aa00108268e5a770c8501dcd638f15c0ded6eb9 ci: add -Wunused to CI configs: apparently -Wunused does not actually include -Wno-unused-parameter:

diff --git a/include/mp/util.h b/include/mp/util.h
index adc7cfb..3074201 100644
--- a/include/mp/util.h
+++ b/include/mp/util.h
@@ -291,11 +291,11 @@ std::string LogEscape(const kj::StringTree& string, size_t max_size);
 
 using Stream = kj::Own<kj::AsyncIoStream>;
 
 using ProcessId = int;
 using SocketId = int;
-inline constexpr SocketId SocketError{-1};
+constexpr SocketId SocketError{-1};
 
 //! Information about parent process passed to child process as a command-line
 //! argument. On unix this is the child socket fd number formatted as a string.
 using SpawnConnectInfo = std::string;

This is only caught with mp::SocketError defined but not used if you bring -Wno-unused-parameter back (level 1 vs 2):

https://gcc.gnu.org/onlinedocs/gcc-15.2.0/gcc/Warning-Options.html#index-Wunused-const-variable

fanquake commented at 10:55 AM on October 30, 2026:

Given the link I guess you mean -Wunused-const-variable here? Adding -Wno-unused-parameter wouldn't produce any warnings.

fanquake force-pushed on Oct 30, 2026

fanquake commented at 11:02 AM on October 30, 2026: member

#371 was merged, can you rebase?

Rebased, added -Wunused-const-variable and addressed comments.