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

fanquake · fanquake · commit 01e1627e25bc · 2022-08-31T08:38:24.000+01:00
fa87534 Fix iwyu (MacroFake) faad673 Fix issues when calling std::move(const&) (MacroFake) Pull request description: 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` ACKs for top commit: ryanofsky: Code review ACK fa87534. Looks good. Good for univalue to support c++11 move optimizations Tree-SHA512: 3dc5cad55b93cfa311abedfb811f35fc1b7f30a1c68561f15942438916c7de25e179c364be11881e01f844f9c2ccd71a3be55967ad5abd2f35b10bb7a882edea
diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh
@@ -65,6 +65,7 @@ if [ "${RUN_TIDY}" = "true" ]; then
           " src/util/serfloat.cpp"\
           " src/util/spanparsing.cpp"\
           " src/util/strencodings.cpp"\
+          " src/util/string.cpp"\
           " src/util/syserror.cpp"\
           " src/util/url.cpp"\
           " -p . ${MAKEJOBS} -- -Xiwyu --cxx17ns -Xiwyu --mapping_file=${BASE_BUILD_DIR}/bitcoin-$HOST/contrib/devtools/iwyu/bitcoin.core.imp"
diff --git a/src/.clang-tidy b/src/.clang-tidy
@@ -6,6 +6,7 @@ misc-unused-using-decls,
 modernize-use-default-member-init,
 modernize-use-nullptr,
 performance-for-range-copy,
+performance-move-const-arg,
 performance-unnecessary-copy-initialization,
 readability-redundant-declaration,
 readability-redundant-string-init,
@@ -17,7 +18,11 @@ misc-unused-using-decls,
 modernize-use-default-member-init,
 modernize-use-nullptr,
 performance-for-range-copy,
+performance-move-const-arg,
 performance-unnecessary-copy-initialization,
 readability-redundant-declaration,
 readability-redundant-string-init,
 '
+CheckOptions:
+ - key: performance-move-const-arg.CheckTriviallyCopyableMove
+   value: false
diff --git a/src/node/mempool_args.cpp b/src/node/mempool_args.cpp
@@ -12,6 +12,7 @@
 #include <logging.h>
 #include <policy/feerate.h>
 #include <policy/policy.h>
+#include <script/standard.h>
 #include <tinyformat.h>
 #include <util/error.h>
 #include <util/moneystr.h>
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
@@ -2129,7 +2129,7 @@ static RPCHelpMan scantxoutset()
         for (const UniValue& scanobject : request.params[1].get_array().getValues()) {
             FlatSigningProvider provider;
             auto scripts = EvalDescriptorStringOrObject(scanobject, provider);
-            for (const auto& script : scripts) {
+            for (CScript& script : scripts) {
                 std::string inferred = InferDescriptor(script, provider)->ToString();
                 needles.emplace(script);
                 descriptors.emplace(std::move(script), std::move(inferred));
diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp
@@ -572,7 +572,7 @@ class DescriptorImpl : public Descriptor
             if (pos++) ret += ",";
             std::string tmp;
             if (!scriptarg->ToStringHelper(arg, tmp, type, cache)) return false;
-            ret += std::move(tmp);
+            ret += tmp;
         }
         return true;
     }
@@ -596,7 +596,7 @@ class DescriptorImpl : public Descriptor
                     tmp = pubkey->ToString();
                     break;
             }
-            ret += std::move(tmp);
+            ret += tmp;
         }
         std::string subscript;
         if (!ToStringSubScriptHelper(arg, subscript, type, cache)) return false;
@@ -912,7 +912,7 @@ class TRDescriptor final : public DescriptorImpl
             }
             std::string tmp;
             if (!m_subdescriptor_args[pos]->ToStringHelper(arg, tmp, type, cache)) return false;
-            ret += std::move(tmp);
+            ret += tmp;
             while (!path.empty() && path.back()) {
                 if (path.size() > 1) ret += '}';
                 path.pop_back();
diff --git a/src/test/fuzz/txorphan.cpp b/src/test/fuzz/txorphan.cpp
@@ -19,7 +19,6 @@
 #include <util/check.h>
 #include <util/time.h>
 
-#include <algorithm>
 #include <cstdint>
 #include <memory>
 #include <set>
diff --git a/src/threadinterrupt.h b/src/threadinterrupt.h
@@ -6,6 +6,7 @@
 #define BITCOIN_THREADINTERRUPT_H
 
 #include <sync.h>
+#include <threadsafety.h>
 
 #include <atomic>
 #include <chrono>
diff --git a/src/univalue/include/univalue.h b/src/univalue/include/univalue.h
@@ -80,14 +80,14 @@ class UniValue {
     bool isArray() const { return (typ == VARR); }
     bool isObject() const { return (typ == VOBJ); }
 
-    void push_back(const UniValue& val);
+    void push_back(UniValue val);
     void push_backV(const std::vector<UniValue>& vec);
     template <class It>
     void push_backV(It first, It last);
 
-    void __pushKV(const std::string& key, const UniValue& val);
-    void pushKV(const std::string& key, const UniValue& val);
-    void pushKVs(const UniValue& obj);
+    void __pushKV(std::string key, UniValue val);
+    void pushKV(std::string key, UniValue val);
+    void pushKVs(UniValue obj);
 
     std::string write(unsigned int prettyIndent = 0,
                       unsigned int indentLevel = 0) const;
diff --git a/src/univalue/lib/univalue.cpp b/src/univalue/lib/univalue.cpp
@@ -101,11 +101,11 @@ void UniValue::setObject()
     typ = VOBJ;
 }
 
-void UniValue::push_back(const UniValue& val_)
+void UniValue::push_back(UniValue val)
 {
     checkType(VARR);
 
-    values.push_back(val_);
+    values.push_back(std::move(val));
 }
 
 void UniValue::push_backV(const std::vector<UniValue>& vec)
@@ -115,32 +115,32 @@ void UniValue::push_backV(const std::vector<UniValue>& vec)
     values.insert(values.end(), vec.begin(), vec.end());
 }
 
-void UniValue::__pushKV(const std::string& key, const UniValue& val_)
+void UniValue::__pushKV(std::string key, UniValue val)
 {
     checkType(VOBJ);
 
-    keys.push_back(key);
-    values.push_back(val_);
+    keys.push_back(std::move(key));
+    values.push_back(std::move(val));
 }
 
-void UniValue::pushKV(const std::string& key, const UniValue& val_)
+void UniValue::pushKV(std::string key, UniValue val)
 {
     checkType(VOBJ);
 
     size_t idx;
     if (findKey(key, idx))
-        values[idx] = val_;
+        values[idx] = std::move(val);
     else
-        __pushKV(key, val_);
+        __pushKV(std::move(key), std::move(val));
 }
 
-void UniValue::pushKVs(const UniValue& obj)
+void UniValue::pushKVs(UniValue obj)
 {
     checkType(VOBJ);
     obj.checkType(VOBJ);
 
     for (size_t i = 0; i < obj.keys.size(); i++)
-        __pushKV(obj.keys[i], obj.values.at(i));
+        __pushKV(std::move(obj.keys.at(i)), std::move(obj.values.at(i)));
 }
 
 void UniValue::getObjMap(std::map<std::string,UniValue>& kv) const
diff --git a/src/util/bip32.cpp b/src/util/bip32.cpp
@@ -6,12 +6,10 @@
 #include <util/bip32.h>
 #include <util/strencodings.h>
 
-#include <algorithm>
 #include <cstdint>
 #include <cstdio>
 #include <sstream>
 
-
 bool ParseHDKeypath(const std::string& keypath_str, std::vector<uint32_t>& keypath)
 {
     std::stringstream ss(keypath_str);
diff --git a/src/util/message.cpp b/src/util/message.cpp
@@ -8,7 +8,6 @@
 #include <key_io.h>
 #include <pubkey.h>
 #include <script/standard.h>
-#include <serialize.h>
 #include <uint256.h>
 #include <util/message.h>
 #include <util/strencodings.h>
diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp
@@ -6,7 +6,6 @@
 #include <span.h>
 #include <util/strencodings.h>
 
-#include <algorithm>
 #include <array>
 #include <cassert>
 #include <cstring>
diff --git a/src/util/string.cpp b/src/util/string.cpp
@@ -6,10 +6,9 @@
 
 #include <regex>
 #include <string>
-#include <utility>
 
 void ReplaceAll(std::string& in_out, const std::string& search, const std::string& substitute)
 {
     if (search.empty()) return;
-    in_out = std::regex_replace(in_out, std::regex(std::move(search)), substitute);
+    in_out = std::regex_replace(in_out, std::regex(search), substitute);
 }
diff --git a/src/util/string.h b/src/util/string.h
@@ -7,7 +7,6 @@
 
 #include <util/spanparsing.h>
 
-#include <algorithm>
 #include <array>
 #include <cstdint>
 #include <cstring>
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
@@ -353,7 +353,7 @@ std::map<CTxDestination, std::vector<COutput>> ListCoins(const CWallet& wallet)
 
     std::map<CTxDestination, std::vector<COutput>> result;
 
-    for (const COutput& coin : AvailableCoinsListUnspent(wallet).All()) {
+    for (COutput& coin : AvailableCoinsListUnspent(wallet).All()) {
         CTxDestination address;
         if ((coin.spendable || (wallet.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && coin.solvable)) &&
             ExtractDestination(FindNonChangeParentOutput(wallet, coin.outpoint).scriptPubKey, address)) {

Original file line number	Diff line number	Diff line change
`@@ -572,7 +572,7 @@ class DescriptorImpl : public Descriptor`
`572`	`572`	`if (pos++) ret += ",";`
`573`	`573`	`std::string tmp;`
`574`	`574`	`if (!scriptarg->ToStringHelper(arg, tmp, type, cache)) return false;`
`575`		`- ret += std::move(tmp);`
	`575`	`+ ret += tmp;`
`576`	`576`	`}`
`577`	`577`	`return true;`
`578`	`578`	`}`
`@@ -596,7 +596,7 @@ class DescriptorImpl : public Descriptor`
`596`	`596`	`tmp = pubkey->ToString();`
`597`	`597`	`break;`
`598`	`598`	`}`
`599`		`- ret += std::move(tmp);`
	`599`	`+ ret += tmp;`
`600`	`600`	`}`
`601`	`601`	`std::string subscript;`
`602`	`602`	`if (!ToStringSubScriptHelper(arg, subscript, type, cache)) return false;`
`@@ -912,7 +912,7 @@ class TRDescriptor final : public DescriptorImpl`
`912`	`912`	`}`
`913`	`913`	`std::string tmp;`
`914`	`914`	`if (!m_subdescriptor_args[pos]->ToStringHelper(arg, tmp, type, cache)) return false;`
`915`		`- ret += std::move(tmp);`
	`915`	`+ ret += tmp;`
`916`	`916`	`while (!path.empty() && path.back()) {`
`917`	`917`	`if (path.size() > 1) ret += '}';`
`918`	`918`	`path.pop_back();`
Original file line number	Diff line number	Diff line change
`@@ -101,11 +101,11 @@ void UniValue::setObject()`
`101`	`101`	`typ = VOBJ;`
`102`	`102`	`}`
`103`	`103`
`104`		`-void UniValue::push_back(const UniValue& val_)`
	`104`	`+void UniValue::push_back(UniValue val)`
`105`	`105`	`{`
`106`	`106`	`checkType(VARR);`
`107`	`107`
`108`		`- values.push_back(val_);`
	`108`	`+ values.push_back(std::move(val));`
`109`	`109`	`}`
`110`	`110`
`111`	`111`	`void UniValue::push_backV(const std::vector<UniValue>& vec)`
`@@ -115,32 +115,32 @@ void UniValue::push_backV(const std::vector<UniValue>& vec)`
`115`	`115`	`values.insert(values.end(), vec.begin(), vec.end());`
`116`	`116`	`}`
`117`	`117`
`118`		`-void UniValue::__pushKV(const std::string& key, const UniValue& val_)`
	`118`	`+void UniValue::__pushKV(std::string key, UniValue val)`
`119`	`119`	`{`
`120`	`120`	`checkType(VOBJ);`
`121`	`121`
`122`		`- keys.push_back(key);`
`123`		`- values.push_back(val_);`
	`122`	`+ keys.push_back(std::move(key));`
	`123`	`+ values.push_back(std::move(val));`
`124`	`124`	`}`
`125`	`125`
`126`		`-void UniValue::pushKV(const std::string& key, const UniValue& val_)`
	`126`	`+void UniValue::pushKV(std::string key, UniValue val)`
`127`	`127`	`{`
`128`	`128`	`checkType(VOBJ);`
`129`	`129`
`130`	`130`	`size_t idx;`
`131`	`131`	`if (findKey(key, idx))`
`132`		`- values[idx] = val_;`
	`132`	`+ values[idx] = std::move(val);`
`133`	`133`	`else`
`134`		`- __pushKV(key, val_);`
	`134`	`+ __pushKV(std::move(key), std::move(val));`
`135`	`135`	`}`
`136`	`136`
`137`		`-void UniValue::pushKVs(const UniValue& obj)`
	`137`	`+void UniValue::pushKVs(UniValue obj)`
`138`	`138`	`{`
`139`	`139`	`checkType(VOBJ);`
`140`	`140`	`obj.checkType(VOBJ);`
`141`	`141`
`142`	`142`	`for (size_t i = 0; i < obj.keys.size(); i++)`
`143`		`- __pushKV(obj.keys[i], obj.values.at(i));`
	`143`	`+ __pushKV(std::move(obj.keys.at(i)), std::move(obj.values.at(i)));`
`144`	`144`	`}`
`145`	`145`
`146`	`146`	`void UniValue::getObjMap(std::map<std::string,UniValue>& kv) const`
Original file line number	Diff line number	Diff line change
`@@ -6,10 +6,9 @@`
`6`	`6`
`7`	`7`	`#include <regex>`
`8`	`8`	`#include <string>`
`9`		`-#include <utility>`
`10`	`9`
`11`	`10`	`void ReplaceAll(std::string& in_out, const std::string& search, const std::string& substitute)`
`12`	`11`	`{`
`13`	`12`	`if (search.empty()) return;`
`14`		`- in_out = std::regex_replace(in_out, std::regex(std::move(search)), substitute);`
	`13`	`+ in_out = std::regex_replace(in_out, std::regex(search), substitute);`
`15`	`14`	`}`