Skip to content

Commit 0beeb91

Browse files
MacroFakeknst
authored andcommitted
Merge 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 bitcoin#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
1 parent 7601680 commit 0beeb91

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
@@ -554,7 +554,6 @@ CScript BuildScript(Ts&&... inputs)
554554
int cnt{0};
555555

556556
([&ret, &cnt] (Ts&& input) {
557-
cnt++;
558557
if constexpr (std::is_same_v<std::remove_cv_t<std::remove_reference_t<Ts>>, CScript>) {
559558
// If it is a CScript, extend ret with it. Move or copy the first element instead.
560559
if (cnt == 0) {
@@ -566,6 +565,7 @@ CScript BuildScript(Ts&&... inputs)
566565
// Otherwise invoke CScript::operator<<.
567566
ret << input;
568567
}
568+
cnt++;
569569
} (std::forward<Ts>(inputs)), ...);
570570

571571
return ret;

0 commit comments

Comments
 (0)