All utilities use the same gArgs global that the node uses. This is generally fine and does not lead to issues unless a bench test is going to spin up a NodeContext via the TestingSetup. In that case the two uses of gArgs conflict and currently it needs to be cleared:
One solution would be to do nothing, because the current code works with that workaround. Another solution would be to not use the same global in all binaries.
-BEGIN VERIFY SCRIPT-
sed -i -e 's/gArgs/argsman/g' src/bench/bench_bitcoin.cpp
-END VERIFY SCRIPT-
8@@ -9,7 +9,7 @@
9 #include <string>
10 #include <vector>
11
12-class UniValue;
13+#include <univalue.h> // For util::SettingsValue = UniValue
But include just in src/bench/bench_bitcoin.cpp?
Yes, that works, but it feels weird to require to include implementation details of one class on the module that ends up happening to use the class.
re: #18662 (review)
Reason is that SettingsValue is a return value of one of the methods of ArgsManager. Not sure why C++ doesn’t require the struct layout for return values of methods when no object with such method is initialized. Though it does require the layout when an object with such method is instantiated… I can’t change the way C++ works.
Complier error messages here are pretty unhelfpul, but this happens because of the ArgsManager destructor, not the method return value. Following change should fix:
0diff --git a/src/util/settings.h b/src/util/settings.h
1index bbb6abe2c09..1d03639fa26 100644
2--- a/src/util/settings.h
3+++ b/src/util/settings.h
4@@ -9,7 +9,7 @@
5 #include <string>
6 #include <vector>
7
8-#include <univalue.h> // For util::SettingsValue = UniValue
9+class UniValue;
10
11 namespace util {
12
13diff --git a/src/util/system.cpp b/src/util/system.cpp
14index b0a538b5277..49a3ab447b9 100644
15--- a/src/util/system.cpp
16+++ b/src/util/system.cpp
17@@ -231,6 +231,8 @@ ArgsManager::ArgsManager()
18 // nothing to do
19 }
20
21+ArgsManager::~ArgsManager() {}
22+
23 const std::set<std::string> ArgsManager::GetUnsuitableSectionOnlyArgs() const
24 {
25 std::set<std::string> unsuitables;
26diff --git a/src/util/system.h b/src/util/system.h
27index 3138522b5c8..96f51e6074a 100644
28--- a/src/util/system.h
29+++ b/src/util/system.h
30@@ -192,6 +192,7 @@ protected:
31
32 public:
33 ArgsManager();
34+ ~ArgsManager();
35
36 /**
37 * Select the network in use
Concept ACK
On behalf of future generations on Bitcoin Core developers: thanks for doing these cleanups - that will make their lives easier :)
The short-term gain from each individual clean-up might seem small but the cumulative long-term gain will make a big difference.