Skip to content

Fix string_arg when used with rref - #110

Merged
arun11299 merged 1 commit into
arun11299:masterfrom
lunacd:forwarding
Apr 22, 2025
Merged

Fix string_arg when used with rref#110
arun11299 merged 1 commit into
arun11299:masterfrom
lunacd:forwarding

Conversation

@lunacd

Copy link
Copy Markdown
Contributor

When passing in a rvalue reference, compiler considers it ambiguous between std::string and std::string&&. Making one of them take a lvalue reference makes compilers correctly pick the right one depending on whether the passed in value binds to a rvalue or lvalue reference.

When passing in a rvalue reference, compiler
considers it ambiguous between std::string and
std::string&&. Making one of them take a lvalue
reference makes compilers correctly pick the right
one depending on whether the passed in value binds
to a rvalue or lvalue reference.
@lunacd

Copy link
Copy Markdown
ContributorAuthor

Minimal repro of what I was saying above: https://godbolt.org/z/naK7oM76v

@arun11299
arun11299 merged commit 2d8a8ee into arun11299:masterApr 22, 2025
hebasto added a commit to hebasto/bitcoin that referenced this pull request Apr 22, 2025
@lunacd
lunacd deleted the forwarding branch April 24, 2025 22:17
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request Apr 27, 2025
When passing in a rvalue reference, compiler
considers it ambiguous between std::string and
std::string&&. Making one of them take a lvalue
reference makes compilers correctly pick the right
one depending on whether the passed in value binds
to a rvalue or lvalue reference.
Github-Pull: arun11299/cpp-subprocess#110
Rebased-From: 2d8a8eebb03e509840e2c3c755d1abf32d930f33
hebasto pushed a commit to hebasto/bitcoin that referenced this pull request May 1, 2025
When passing in a rvalue reference, compiler
considers it ambiguous between std::string and
std::string&&. Making one of them take a lvalue
reference makes compilers correctly pick the right
one depending on whether the passed in value binds
to a rvalue or lvalue reference.
Github-Pull: arun11299/cpp-subprocess#110
Rebased-From: 2d8a8eebb03e509840e2c3c755d1abf32d930f33
hebasto added a commit to bitcoin/bitcoin that referenced this pull request May 5, 2025
cd95c9d subprocess: check and handle fcntl(F_GETFD) failure (Tomás Andróil)
b7288de subprocess: Proper implementation of wait() on Windows (Haowen Liu)
7423214 subprocess: Do not escape double quotes for command line arguments on Windows (Hennadii Stepanov)
bb9ffea subprocess: Explicitly define move constructor of Streams class (Shunsuke Shimizu)
174bd43 subprocess: Avoid leaking POSIX name aliases beyond `subprocess.h` (Hennadii Stepanov)
7997b76 subprocess: Fix cross-compiling with mingw toolchain (Hennadii Stepanov)
6476304 subprocess: Get Windows return code in wait() (Haowen Liu)
d3f511b subprocess: Fix string_arg when used with rref (Haowen Liu)
2fd3f2f subprocess: Fix memory leaks (Haoran Peng)
Pull request description:
Most of these changes were developed during work on #29868 and #32342 and have since been upstreamed.
As they are now merged, this PR backports them to our `src/util/subprocess.h` header.
Required for #29868.
A list of the backported PRs:
- arun11299/cpp-subprocess#106
- arun11299/cpp-subprocess#110
- arun11299/cpp-subprocess#109
- arun11299/cpp-subprocess#99
- arun11299/cpp-subprocess#112
- arun11299/cpp-subprocess#107
- arun11299/cpp-subprocess#113
- arun11299/cpp-subprocess#116
- arun11299/cpp-subprocess#117
The following PRs were skipped for backporting:
- arun11299/cpp-subprocess#108 because we are not planning to support this feature.
- arun11299/cpp-subprocess#101 because that change has been already landed in #29849.
ACKs for top commit:
theStack:
Light ACK cd95c9d
laanwj:
Code review re-ACK cd95c9d
Tree-SHA512: f9b60b932957d2e1cad1d87f2ad8bb68c97136e9735eb78547018a42cc50c4652750367f29462eadb0512c27db1dd8a7d4b17a2f0aeab62b3dbf86db5f51a61c
alexanderwiederin pushed a commit to alexanderwiederin/bitcoin that referenced this pull request May 8, 2025
When passing in a rvalue reference, compiler
considers it ambiguous between std::string and
std::string&&. Making one of them take a lvalue
reference makes compilers correctly pick the right
one depending on whether the passed in value binds
to a rvalue or lvalue reference.
Github-Pull: arun11299/cpp-subprocess#110
Rebased-From: 2d8a8eebb03e509840e2c3c755d1abf32d930f33
luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Jun 6, 2025
When passing in a rvalue reference, compiler
considers it ambiguous between std::string and
std::string&&. Making one of them take a lvalue
reference makes compilers correctly pick the right
one depending on whether the passed in value binds
to a rvalue or lvalue reference.
Github-Pull: arun11299/cpp-subprocess#110
Rebased-From: 2d8a8eebb03e509840e2c3c755d1abf32d930f33
Github-Pull: bitcoin#32358
Rebased-From: d3f511b
janus pushed a commit to BitgesellOfficial/bitgesell that referenced this pull request Sep 7, 2025
When passing in a rvalue reference, compiler
considers it ambiguous between std::string and
std::string&&. Making one of them take a lvalue
reference makes compilers correctly pick the right
one depending on whether the passed in value binds
to a rvalue or lvalue reference.
Github-Pull: arun11299/cpp-subprocess#110
Rebased-From: 2d8a8eebb03e509840e2c3c755d1abf32d930f33
Kino1994 pushed a commit to Kino1994/bitcoin-full-history that referenced this pull request Jun 28, 2026
When passing in a rvalue reference, compiler
considers it ambiguous between std::string and
std::string&&. Making one of them take a lvalue
reference makes compilers correctly pick the right
one depending on whether the passed in value binds
to a rvalue or lvalue reference.
Github-Pull: arun11299/cpp-subprocess#110
Rebased-From: 2d8a8eebb03e509840e2c3c755d1abf32d930f33
Kino1994 pushed a commit to Kino1994/bitcoin-full-history that referenced this pull request Jun 28, 2026
b5f498d subprocess: check and handle fcntl(F_GETFD) failure (Tomás Andróil)
2117d6e subprocess: Proper implementation of wait() on Windows (Haowen Liu)
d281ca6 subprocess: Do not escape double quotes for command line arguments on Windows (Hennadii Stepanov)
a322a8b subprocess: Explicitly define move constructor of Streams class (Shunsuke Shimizu)
8e5bb08 subprocess: Avoid leaking POSIX name aliases beyond `subprocess.h` (Hennadii Stepanov)
37421cd subprocess: Fix cross-compiling with mingw toolchain (Hennadii Stepanov)
f2006c7 subprocess: Get Windows return code in wait() (Haowen Liu)
7630b96 subprocess: Fix string_arg when used with rref (Haowen Liu)
ff0df49 subprocess: Fix memory leaks (Haoran Peng)
Pull request description:
Most of these changes were developed during work on bitcoin/bitcoin#29868 and bitcoin/bitcoin#32342 and have since been upstreamed.
As they are now merged, this PR backports them to our `src/util/subprocess.h` header.
Required for bitcoin/bitcoin#29868.
A list of the backported PRs:
- arun11299/cpp-subprocess#106
- arun11299/cpp-subprocess#110
- arun11299/cpp-subprocess#109
- arun11299/cpp-subprocess#99
- arun11299/cpp-subprocess#112
- arun11299/cpp-subprocess#107
- arun11299/cpp-subprocess#113
- arun11299/cpp-subprocess#116
- arun11299/cpp-subprocess#117
The following PRs were skipped for backporting:
- arun11299/cpp-subprocess#108 because we are not planning to support this feature.
- arun11299/cpp-subprocess#101 because that change has been already landed in bitcoin/bitcoin#29849.
ACKs for top commit:
theStack:
Light ACK b5f498d
laanwj:
Code review re-ACK b5f498d
Tree-SHA512: f9b60b932957d2e1cad1d87f2ad8bb68c97136e9735eb78547018a42cc50c4652750367f29462eadb0512c27db1dd8a7d4b17a2f0aeab62b3dbf86db5f51a61c
BigcoinBGC pushed a commit to BigcoinBGC/bigcoin that referenced this pull request Jun 30, 2026
When passing in a rvalue reference, compiler
considers it ambiguous between std::string and
std::string&&. Making one of them take a lvalue
reference makes compilers correctly pick the right
one depending on whether the passed in value binds
to a rvalue or lvalue reference.
Github-Pull: arun11299/cpp-subprocess#110
Rebased-From: 2d8a8eebb03e509840e2c3c755d1abf32d930f33
BigcoinBGC pushed a commit to BigcoinBGC/bigcoin that referenced this pull request Jun 30, 2026
3a0f4d0 subprocess: check and handle fcntl(F_GETFD) failure (Tomás Andróil)
5ec84fb subprocess: Proper implementation of wait() on Windows (Haowen Liu)
a31139a subprocess: Do not escape double quotes for command line arguments on Windows (Hennadii Stepanov)
c006067 subprocess: Explicitly define move constructor of Streams class (Shunsuke Shimizu)
a8f4858 subprocess: Avoid leaking POSIX name aliases beyond `subprocess.h` (Hennadii Stepanov)
c1c6401 subprocess: Fix cross-compiling with mingw toolchain (Hennadii Stepanov)
c5478f3 subprocess: Get Windows return code in wait() (Haowen Liu)
fee2782 subprocess: Fix string_arg when used with rref (Haowen Liu)
b5de4db subprocess: Fix memory leaks (Haoran Peng)
Pull request description:
Most of these changes were developed during work on bitcoin/bitcoin#29868 and bitcoin/bitcoin#32342 and have since been upstreamed.
As they are now merged, this PR backports them to our `src/util/subprocess.h` header.
Required for bitcoin/bitcoin#29868.
A list of the backported PRs:
- arun11299/cpp-subprocess#106
- arun11299/cpp-subprocess#110
- arun11299/cpp-subprocess#109
- arun11299/cpp-subprocess#99
- arun11299/cpp-subprocess#112
- arun11299/cpp-subprocess#107
- arun11299/cpp-subprocess#113
- arun11299/cpp-subprocess#116
- arun11299/cpp-subprocess#117
The following PRs were skipped for backporting:
- arun11299/cpp-subprocess#108 because we are not planning to support this feature.
- arun11299/cpp-subprocess#101 because that change has been already landed in bitcoin/bitcoin#29849.
ACKs for top commit:
theStack:
Light ACK 3a0f4d0
laanwj:
Code review re-ACK 3a0f4d0
Tree-SHA512: f9b60b932957d2e1cad1d87f2ad8bb68c97136e9735eb78547018a42cc50c4652750367f29462eadb0512c27db1dd8a7d4b17a2f0aeab62b3dbf86db5f51a61c
Kino1994 pushed a commit to Kino1994/bitcoin-full-history that referenced this pull request Aug 19, 2026
When passing in a rvalue reference, compiler
considers it ambiguous between std::string and
std::string&&. Making one of them take a lvalue
reference makes compilers correctly pick the right
one depending on whether the passed in value binds
to a rvalue or lvalue reference.
Github-Pull: arun11299/cpp-subprocess#110
Rebased-From: 2d8a8eebb03e509840e2c3c755d1abf32d930f33
Kino1994 pushed a commit to Kino1994/bitcoin-full-history that referenced this pull request Aug 19, 2026
cd95c9d6a7ec08cca0f9c98328c759be586720f8 subprocess: check and handle fcntl(F_GETFD) failure (Tomás Andróil)
b7288decdf534391b3f917cfb11ec62580407c3f subprocess: Proper implementation of wait() on Windows (Haowen Liu)
7423214d8deebbbcdab66090e185129decfaa799 subprocess: Do not escape double quotes for command line arguments on Windows (Hennadii Stepanov)
bb9ffea53fb021580f069c431aee02f547039831 subprocess: Explicitly define move constructor of Streams class (Shunsuke Shimizu)
174bd43f2e46a0ccc6f5ad486bb587c72c1241c3 subprocess: Avoid leaking POSIX name aliases beyond `subprocess.h` (Hennadii Stepanov)
7997b7656f99c5415cfa02cb1206226d0a82efd6 subprocess: Fix cross-compiling with mingw toolchain (Hennadii Stepanov)
647630462f10cacc7a75da7c82ca7c6d33bbde4b subprocess: Get Windows return code in wait() (Haowen Liu)
d3f511b4583b3771bf941ebc7884477430115039 subprocess: Fix string_arg when used with rref (Haowen Liu)
2fd3f2fec67a3bb62378c286fbf9667e6fb3cc3b subprocess: Fix memory leaks (Haoran Peng)
Pull request description:
Most of these changes were developed during work on bitcoin/bitcoin#29868 and bitcoin/bitcoin#32342 and have since been upstreamed.
As they are now merged, this PR backports them to our `src/util/subprocess.h` header.
Required for bitcoin/bitcoin#29868.
A list of the backported PRs:
- arun11299/cpp-subprocess#106
- arun11299/cpp-subprocess#110
- arun11299/cpp-subprocess#109
- arun11299/cpp-subprocess#99
- arun11299/cpp-subprocess#112
- arun11299/cpp-subprocess#107
- arun11299/cpp-subprocess#113
- arun11299/cpp-subprocess#116
- arun11299/cpp-subprocess#117
The following PRs were skipped for backporting:
- arun11299/cpp-subprocess#108 because we are not planning to support this feature.
- arun11299/cpp-subprocess#101 because that change has been already landed in bitcoin/bitcoin#29849.
ACKs for top commit:
theStack:
Light ACK cd95c9d6a7ec08cca0f9c98328c759be586720f8
laanwj:
Code review re-ACK cd95c9d6a7ec08cca0f9c98328c759be586720f8
Tree-SHA512: f9b60b932957d2e1cad1d87f2ad8bb68c97136e9735eb78547018a42cc50c4652750367f29462eadb0512c27db1dd8a7d4b17a2f0aeab62b3dbf86db5f51a61c
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@lunacd@arun11299