Skip to content

util: remove unused exception parameter in Popen::execute_process - #36383

Merged
hebasto merged 2 commits into
bitcoin:masterfrom
fanquake:unused_exception_param
Oct 2, 2026
Merged

hebasto merged 2 commits into
bitcoin:masterfrom
fanquake:unused_exception_param

Conversation

@fanquake

@fanquake fanquake commented Sep 29, 2026 •

Copy link
Copy Markdown
Member
[451/1129] Building CXX object src/CMakeFiles/bitcoin_common.dir/common/run_command.cpp.o
In file included from ../src/common/run_command.cpp:10:
../src/util/subprocess.h:1219:30: warning: unused exception parameter 'exp' [-Wunused-exception-parameter]
 1219 |     } catch (std::exception& exp) {
      |                              ^~~
1 warning generated.

Related to #36167.

@DrahtBot

DrahtBot commented Sep 29, 2026 •

Copy link
Copy Markdown
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/36383.

Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers
ACK musaHaruna, maflcko, willcl-ark, hebasto

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #36167 ([RFC] Enable -Wunused by fanquake)

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

Copy link
Copy Markdown
Member Author

Upstreamed in arun11299/cpp-subprocess#133.

@fanquake
fanquake force-pushed the unused_exception_param branch from f3d730f to cde2f2e Compare September 30, 2026 09:19
@fanquake
fanquake requested a review from hebasto October 1, 2026 09:22

@musaHaruna musaHaruna left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK cde2f2e

Checked that removing exp leaves the catch type, cleanup call, and throw; unchanged.

Built on macOS arm64 with Apple Clang 17, with -Wunused-exception-parameter and -Werror enabled. Confirmed the original header triggers the warning and the updated header compiles successfully.

@maflcko

maflcko commented Oct 1, 2026

Copy link
Copy Markdown
Member

Built on macOS arm64 with Apple Clang 17

Unrelated, but I see you are compiling with Apple Clang 17. I wonder which exact version, because in #36287 I bump to the latest Apple Clang 17 version. So I wonder if this works for you, or if you'd have to upgrade your macOS and/or XCode.

@maflcko

maflcko commented Oct 1, 2026

Copy link
Copy Markdown
Member

lgtm ACK cde2f2e

Seems fine to enforce this as a small, mostly-style preference.

@musaHaruna

Copy link
Copy Markdown
Contributor

Unrelated, but I see you are compiling with Apple Clang 17. I wonder which exact version, because in #36287 I bump to the latest Apple Clang 17 version. So I wonder if this works for you, or if you'd have to upgrade your macOS and/or XCode.

I’m using Apple Clang 17.0.0 (clang-1700.6.3.2) from Command Line Tools 26.2, on macOS 26.0 (arm64). I haven't looked at #36287 yet, but I will take a look.

@willcl-ark willcl-ark left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK cde2f2e

@hebasto hebasto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK cde2f2e.

@hebasto
hebasto merged commit 69142ea into bitcoin:master Oct 2, 2026
27 checks passed
@fanquake
fanquake deleted the unused_exception_param branch October 2, 2026 13:15
Kino1994 pushed a commit to Kino1994/bitcoin-full-history that referenced this pull request Oct 5, 2026
…in `Popen::execute_process`

cde2f2ec641acfb7ab11e5d37182b5fcafb65a94 build: enable -Wunused-exception-parameter (fanquake)
1b00dff743da029a25b13e31e79552befc4c3f30 util: remove unused exception parameter in Popen::execute_process (fanquake)

Pull request description:

  ```bash
  [451/1129] Building CXX object src/CMakeFiles/bitcoin_common.dir/common/run_command.cpp.o
  In file included from ../src/common/run_command.cpp:10:
  ../src/util/subprocess.h:1219:30: warning: unused exception parameter 'exp' [-Wunused-exception-parameter]
   1219 |     } catch (std::exception& exp) {
        |                              ^~~
  1 warning generated.
  ```

  Related to #36167.

ACKs for top commit:
  maflcko:
    lgtm ACK cde2f2ec641acfb7ab11e5d37182b5fcafb65a94
  musaHaruna:
    ACK [cde2f2e](github.com/bitcoin/bitcoin/pull/36383/changes/cde2f2ec641acfb7ab11e5d37182b5fcafb65a94)
  willcl-ark:
    ACK cde2f2ec641acfb7ab11e5d37182b5fcafb65a94
  hebasto:
    ACK cde2f2ec641acfb7ab11e5d37182b5fcafb65a94.

Tree-SHA512: 85bd84770bf7c8da1837f1940971e6572d80856ec31d0459e3032c0eb94e0c0d7ee1aaea8c51028652577b3f90ba20ffbf9ada7e92eb2f8173b8df74a14c82db
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants