Fix issues when calling std::move(const&) #25872

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:2208-const-move-📔 changing 15 files +28 −27

MarcoFalke commented at 9:12 AM on August 19, 2022: member
Passing a symbol to std::move that is marked const is a no-op, which can be fixed in two ways:
- Remove the const, or
- Remove the std::move
MarcoFalke added the label Refactoring on Aug 19, 2022
MarcoFalke force-pushed on Aug 19, 2022
MarcoFalke force-pushed on Aug 19, 2022
MarcoFalke force-pushed on Aug 19, 2022
MarcoFalke force-pushed on Aug 19, 2022
MarcoFalke force-pushed on Aug 19, 2022
DrahtBot commented at 12:47 PM on August 19, 2022: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #25714 (univalue: Avoid std::string copies by MarcoFalke)
- #24897 ([Draft / POC] Silent Payments by w0xlt)
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.
DrahtBot added the label Needs rebase on Aug 19, 2022
Fix issues when calling std::move(const&) faad673716
Fix iwyu fa875349e2
MarcoFalke force-pushed on Aug 20, 2022
DrahtBot removed the label Needs rebase on Aug 20, 2022
fanquake requested review from ryanofsky on Aug 22, 2022

in src/.clang-tidy:8 in fa875349e2

   4 | @@ -5,6 +5,7 @@ misc-unused-using-decls,
   5 |  modernize-use-default-member-init,
   6 |  modernize-use-nullptr,
   7 |  performance-for-range-copy,
   8 | +performance-move-const-arg,

Sjors commented at 2:27 PM on August 22, 2022:

Good idea to add this.

MarcoFalke commented at 6:43 AM on August 25, 2022: member

Locally, on aarch64, I see that the BlockToJsonVerbose benchmark is about 2% faster. Also, for some unknown reason, the BlockToJsonVerboseWrite bench uses 5% less memory?
ryanofsky approved
ryanofsky commented at 7:16 PM on August 26, 2022: contributor

Code review ACK fa875349e22f2f0f9c2c98ee991372d08ff90318. Looks good. Good for univalue to support c++11 move optimizations
fanquake merged this on Aug 31, 2022
fanquake closed this on Aug 31, 2022
MarcoFalke deleted the branch on Sep 1, 2022
sidhujag referenced this in commit 9600c3bb20 on Sep 1, 2022
PastaPastaPasta referenced this in commit 264de42875 on Oct 18, 2022
bitcoin locked this on Sep 1, 2023

Contributors

Labels

Fix issues when calling std::move(const&) #25872

Conflicts