Merge bitcoin/bitcoin#25709: script: actually trigger the optimization in BuildScript

00897d0677 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 00897d0677
  MarcoFalke:
    review ACK 00897d0677

Tree-SHA512: b575bd444b0cd2fe754ec5f3e2f3f53d2696d5dcebedcace1e38be372c82365e75938dfe185429ed5a83efe1a395e204bfb33efe56c10defc5811eaee50580e3
This commit is contained in:
MacroFake 2022-07-30 17:48:31 +02:00
commit 5215c80edc
No known key found for this signature in database
GPG key ID: CE2B75697E69A548

View file

@ -588,7 +588,6 @@ CScript BuildScript(Ts&&... inputs)
int cnt{0};
([&ret, &cnt] (Ts&& input) {
cnt++;
if constexpr (std::is_same_v<std::remove_cv_t<std::remove_reference_t<Ts>>, CScript>) {
// If it is a CScript, extend ret with it. Move or copy the first element instead.
if (cnt == 0) {
@ -600,6 +599,7 @@ CScript BuildScript(Ts&&... inputs)
// Otherwise invoke CScript::operator<<.
ret << input;
}
cnt++;
} (std::forward<Ts>(inputs)), ...);
return ret;