subprocess: Fix `-Wunused-private-field` when building with clang-cl on Windows

hebasto commented at 10:26 pm on January 22, 2026: member

This PR is a prerequisite for #31507.

It resolves -Wunused-private-field warnings triggered in src/util/subprocess.h when compiling with clang-cl on Windows:

0D:\a\bitcoin\bitcoin\src\util/subprocess.h(759,10): warning : private field 'parent_' is not used [-Wunused-private-field] [D:\a\bitcoin\bitcoin\build\src\util\bitcoin_util.vcxproj]
1D:\a\bitcoin\bitcoin\src\util/subprocess.h(760,7): warning : private field 'err_wr_pipe_' is not used [-Wunused-private-field] [D:\a\bitcoin\bitcoin\build\src\util\bitcoin_util.vcxproj]
2D:\a\bitcoin\bitcoin\src\util/subprocess.h(1038,7): warning : private field 'child_pid_' is not used [-Wunused-private-field] [D:\a\bitcoin\bitcoin\build\src\util\bitcoin_util.vcxproj]

Only the second commit has been submitted upstream. The first commit is specific to this repository and not applicable upstream.

hebasto added the label Refactoring on Jan 22, 2026

hebasto added the label Windows on Jan 22, 2026

DrahtBot commented at 10:26 pm on January 22, 2026: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34385.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	maflcko, purpleKarrot, sedited

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

in src/util/subprocess.h:938 in d39e9d68a6 outdated

933@@ -932,7 +934,9 @@ class Popen
934 {
935 public:
936   friend struct detail::ArgumentDeducer;
937+#ifndef __USING_WINDOWS__
938   friend class detail::Child;

fanquake commented at 10:48 am on January 23, 2026:

Can these be fixed with a few [[maybe_unused]] rather than a bunch of #ifdefs and shuffling code around?

maflcko commented at 12:30 pm on January 23, 2026:

i think it is fine to not compile fork-exec-child on Windows, if it doesn’t exist there conceptually.

hebasto commented at 1:05 pm on January 23, 2026:

The point was to make these changes compatible with the upstream, which uses C++11 standard.

fanquake commented at 12:41 pm on February 17, 2026:

Sure, but it seems a bit odd at this point, to try and remain compatible with upstream, which uses an older standard, is getting little-to-no maintainence, seems to be generally low code quality, and we are continuing to make header changes that diverge.

maflcko commented at 12:43 pm on February 17, 2026:

I still have a slight preference to guard this behind __USING_WINDOWS__. It seems cleaner, but no strong opinion.

fanquake commented at 12:15 pm on January 23, 2026: member

Context and details can be found in #31507 (review).

Can you put the relevent context and details in this PR, and in the commit messages, rather than linking to a comment in a different PR.

subprocess: Fix `-Wunused-private-field` for `Popen` class on Windows

When compiling with clang-cl on Windows, `src/util/subprocess.h` emits
`-Wunused-private-field` warnings about unused private fields in the
`Popen` class.

9f2b338bc0

subprocess: Fix `-Wunused-private-field` for `Child` class on Windows

When compiling with clang-cl on Windows, `src/util/subprocess.h` emits
`-Wunused-private-field` warnings about unused private fields in the
`Child` class.

1b36bf0c5d

hebasto force-pushed on Jan 23, 2026

hebasto commented at 1:42 pm on January 23, 2026: member

Context and details can be found in #31507 (comment).

Can you put the relevent context and details in this PR, and in the commit messages, rather than linking to a comment in a different PR.

Thanks! Rewritten.

maflcko commented at 12:44 pm on February 17, 2026: member

review ACK 1b36bf0c5d71ea2e7e3595eb10611ea1a8c3a0d1 👋

Signature:

0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: review ACK 1b36bf0c5d71ea2e7e3595eb10611ea1a8c3a0d1 👋
3I9U6zcesjW0zlruquf+ALGeAuSFK660ouAJH0/n7Xkf7OQwvGsygHSjtVea/fxa4i0lFllOOqsuco5UOq8kiBQ==

hebasto commented at 8:09 am on February 18, 2026: member

… the second commit has been submitted upstream.

It is landed now.

purpleKarrot commented at 8:33 am on February 18, 2026: contributor

ACK 1b36bf0c5d71ea2e7e3595eb10611ea1a8c3a0d1

Note that the C++ standard reserves names that contain two consecutive underscores, so strictly speaking, this is not valid C++. Unrelated, I know.

maflcko commented at 8:36 am on February 18, 2026: member

Note that the C++ standard reserves names that contain two consecutive underscores, so strictly speaking, this is not valid C++. Unrelated, I know.

Maybe a scripted-diff upstream and here could fix this :sweat_smile:

purpleKarrot commented at 8:43 am on February 18, 2026: contributor

Another thing that comes to mind is that it is weird to conclude the target platform from he compiler identifier:

0#if (defined _MSC_VER) || (defined __MINGW32__)
1  #define __USING_WINDOWS__
2#endif

This code is assuming, that if either MSVC or MINGW are used as the compiler, the target platform must be windows. But why not use _WIN32 directly whenever the win32 API is targeted? It is already set by all compilers.

fanquake commented at 8:53 am on February 18, 2026: member

Yea. Rather than introducing more redundant, technically invalid C++, to stay compatible with upstream, we could just diverge, and write better code.

sedited approved

sedited commented at 3:26 pm on February 19, 2026: contributor

ACK 1b36bf0c5d71ea2e7e3595eb10611ea1a8c3a0d1

At this point we are diverged anyway, so I’d suggest renaming __USING_WINDOWS__ in a follow-up.

sedited merged this on Feb 19, 2026

sedited closed this on Feb 19, 2026

hebasto deleted the branch on Feb 19, 2026

subprocess: Fix -Wunused-private-field when building with clang-cl on Windows #34385

Code Coverage & Benchmarks

Reviews

subprocess: Fix `-Wunused-private-field` when building with clang-cl on Windows #34385