Skip to content

Commit 5215c80

Browse files
author
MacroFake
committed
Merge bitcoin/bitcoin#25709: script: actually trigger the optimization in BuildScript
00897d0 script: actually trigger the optimization in BuildScript (Antoine Poinsot) Pull request description: The counter is an optimization over calling `ret.empty()`. It was suggested that the compiler would realize `cnt` is only `0` on the first iteration, and not actually emit the check and conditional. This optimization was actually not triggered at all, since we incremented `cnt` at the beginning of the first iteration. Fix it by incrementing at the end instead. This was reported by Github user "Janus". Fixes #25682. Note this does *not* change semantics. It only allows the optimization of moving instead of copying on first `CScript` element to actually be reachable. ACKs for top commit: sipa: utACK 00897d0 MarcoFalke: review ACK 00897d0 Tree-SHA512: b575bd444b0cd2fe754ec5f3e2f3f53d2696d5dcebedcace1e38be372c82365e75938dfe185429ed5a83efe1a395e204bfb33efe56c10defc5811eaee50580e3
2 parents 8e37afc + 00897d0 commit 5215c80

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

src/script/script.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -588,7 +588,6 @@ CScript BuildScript(Ts&&... inputs)
588588
int cnt{0};
589589

590590
([&ret, &cnt] (Ts&& input) {
591-
cnt++;
592591
if constexpr (std::is_same_v<std::remove_cv_t<std::remove_reference_t<Ts>>, CScript>) {
593592
// If it is a CScript, extend ret with it. Move or copy the first element instead.
594593
if (cnt == 0) {
@@ -600,6 +599,7 @@ CScript BuildScript(Ts&&... inputs)
600599
// Otherwise invoke CScript::operator<<.
601600
ret << input;
602601
}
602+
cnt++;
603603
} (std::forward<Ts>(inputs)), ...);
604604

605605
return ret;

0 commit comments

Comments
 (0)