tidy: add modernize-use-using #25695

Refactor and introduce modernize-use-using into our clang-tidy job. Given we already use both, consolidate to the later.

See: https://releases.llvm.org/14.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/modernize-use-using.html.

Concept ACK

I see you refactored typedefs in *.cpp files, can it be done in header files as well?

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #26177 (refactor / kernel: Move non-gArgs chainparams functionality to kernel by TheCharlatan)
• #26158 (bench: add "priority level" to the benchmark framework by furszy)
• #25620 (wallet: Introduce AddressBookManager by furszy)
• #25572 (refactor: Introduce EvictionManager and use it for the inbound eviction logic by dergoegge)
• #21590 (Safegcd-based modular inverses in MuHash3072 by sipa)
• #17786 (refactor: Nuke policy/fees->mempool circular dependencies by hebasto)
• #16195 (util: Use void* throughout support/lockedpool.h by jkczyz)

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.

I see you refactored typedefs in *.cpp files, can it be done in header files as well?

Added some changes to header files.

Code review ACK d1885b434f3612ac1efe926e230a440bd7eed366

Rebased for #25707.

I believe you can change these as well.

<details> <summary>Git diff</summary>

diff --git a/src/blockencodings.h b/src/blockencodings.h
index 67c4e5715..700ebb252 100644
--- a/src/blockencodings.h
+++ b/src/blockencodings.h
@@ -74,14 +74,13 @@ struct PrefilledTransaction {
     SERIALIZE_METHODS(PrefilledTransaction, obj) { READWRITE(COMPACTSIZE(obj.index), Using<TransactionCompression>(obj.tx)); }
 };
 
-typedef enum ReadStatus_t
-{
+using ReadStatus = enum ReadStatus_t {
     READ_STATUS_OK,
     READ_STATUS_INVALID, // Invalid object, peer is sending bogus crap
     READ_STATUS_FAILED, // Failed to process object
     READ_STATUS_CHECKBLOCK_FAILED, // Used only by FillBlock to indicate a
                                    // failure in CheckBlock.
-} ReadStatus;
+};
 
 class CBlockHeaderAndShortTxIDs {
 private:
diff --git a/src/protocol.h b/src/protocol.h
index b85dc0d82..af105f90a 100644
--- a/src/protocol.h
+++ b/src/protocol.h
@@ -33,7 +33,7 @@ public:
     static constexpr size_t MESSAGE_SIZE_OFFSET = MESSAGE_START_SIZE + COMMAND_SIZE;
     static constexpr size_t CHECKSUM_OFFSET = MESSAGE_SIZE_OFFSET + MESSAGE_SIZE_SIZE;
     static constexpr size_t HEADER_SIZE = MESSAGE_START_SIZE + COMMAND_SIZE + MESSAGE_SIZE_SIZE + CHECKSUM_SIZE;
-    typedef unsigned char MessageStartChars[MESSAGE_START_SIZE];
+    using MessageStartChars = unsigned char[MESSAGE_START_SIZE];
 
     explicit CMessageHeader() = default;
 
diff --git a/src/rpc/server.h b/src/rpc/server.h
index 01e855605..31fd72ce3 100644
--- a/src/rpc/server.h
+++ b/src/rpc/server.h
@@ -84,7 +84,7 @@ void RPCUnsetTimerInterface(RPCTimerInterface *iface);
  */
 void RPCRunLater(const std::string& name, std::function<void()> func, int64_t nSeconds);
 
-typedef RPCHelpMan (*RpcMethodFnType)();
+using RpcMethodFnType = RPCHelpMan (*)();
 
 class CRPCCommand
 {
diff --git a/src/script/bitcoinconsensus.h b/src/script/bitcoinconsensus.h
index 8fea42e4b..fda5e3c5e 100644
--- a/src/script/bitcoinconsensus.h
+++ b/src/script/bitcoinconsensus.h
@@ -33,7 +33,7 @@ extern "C" {
 
 #define BITCOINCONSENSUS_API_VER 1
 
-typedef enum bitcoinconsensus_error_t
+using bitcoinconsensus_error = enum bitcoinconsensus_error_t
 {
     bitcoinconsensus_ERR_OK = 0,
     bitcoinconsensus_ERR_TX_INDEX,
@@ -41,7 +41,8 @@ typedef enum bitcoinconsensus_error_t
     bitcoinconsensus_ERR_TX_DESERIALIZE,
     bitcoinconsensus_ERR_AMOUNT_REQUIRED,
     bitcoinconsensus_ERR_INVALID_FLAGS,
-} bitcoinconsensus_error;
+};
 
 /** Script verification flags */
 enum
diff --git a/src/script/script_error.h b/src/script/script_error.h
index 44e68fe0f..2674ca8dd 100644
--- a/src/script/script_error.h
+++ b/src/script/script_error.h
@@ -8,7 +8,7 @@
 
 #include <string>
 
-typedef enum ScriptError_t
+using ScriptError = enum ScriptError_t
 {
     SCRIPT_ERR_OK = 0,
     SCRIPT_ERR_UNKNOWN_ERROR,
@@ -83,7 +83,7 @@ typedef enum ScriptError_t
     SCRIPT_ERR_SIG_FINDANDDELETE,
 
     SCRIPT_ERR_ERROR_COUNT
-} ScriptError;
+};
 
 #define SCRIPT_ERR_LAST SCRIPT_ERR_ERROR_COUNT
 
diff --git a/src/support/events.h b/src/support/events.h
index 0a74d0236..24f807ae3 100644
--- a/src/support/events.h
+++ b/src/support/events.h
@@ -19,7 +19,7 @@ struct type##_deleter {\
     }\
 };\
 /* unique ptr typedef */\
-typedef std::unique_ptr<struct type, type##_deleter> raii_##type
+using raii_##type = std::unique_ptr<struct type, type##_deleter>;
 
 MAKE_RAII(event_base);
 MAKE_RAII(event);
diff --git a/src/support/lockedpool.h b/src/support/lockedpool.h
index 1c0d0669b..bbbb5d374 100644
--- a/src/support/lockedpool.h
+++ b/src/support/lockedpool.h
@@ -139,7 +139,8 @@ public:
 
     /** Callback when allocation succeeds but locking fails.
      */
-    typedef bool (*LockingFailed_Callback)();
+    using LockingFailed_Callback = bool (*)();
 
     /** Memory statistics. */
     struct Stats
diff --git a/src/tinyformat.h b/src/tinyformat.h
index bedaa1400..e938ea046 100644
--- a/src/tinyformat.h
+++ b/src/tinyformat.h
@@ -221,7 +221,9 @@ struct is_convertible
 
 
 // Detect when a type is not a wchar_t string
-template<typename T> struct is_wchar { typedef int tinyformat_wchar_is_not_supported; };
+template<typename T> struct is_wchar { 
+    using tinyformat_wchar_is_not_supported = int;
+};
 template<> struct is_wchar<wchar_t*> {};
 template<> struct is_wchar<const wchar_t*> {};
 template<int n> struct is_wchar<const wchar_t[n]> {};
@@ -332,7 +334,7 @@ inline void formatValue(std::ostream& out, const char* /*fmtBegin*/,
 #ifndef TINYFORMAT_ALLOW_WCHAR_STRINGS
     // Since we don't support printing of wchar_t using "%ls", make it fail at
     // compile time in preference to printing as a void* at runtime.
-    typedef typename detail::is_wchar<T>::tinyformat_wchar_is_not_supported DummyType;
+    using DummyType = typename detail::is_wchar<T>::tinyformat_wchar_is_not_supported;
     (void) DummyType(); // avoid unused type warning with gcc-4.8
 #endif
     // The mess here is to support the %c and %p conversions: if these
@@ -958,7 +960,7 @@ class FormatList
 };
 
 /// Reference to type-opaque format list for passing to vformat()
-typedef const FormatList& FormatListRef;
+using FormatListRef = const FormatList&;
 
 
 namespace detail {

</details>

I believe you can change these as well.

Thanks. Incorporated those are that aren't upstream code, i.e tinyformat.

ACK 313241d757c022d978d87e1abd9ab4bb68d45986

What is the point of this other than causing merge conflicts, given that the linter doesn't even enforce any of this in header files?

I mean it is great that we have clang-tidy but we should use it for transformations/checks that prevent bugs, not as an excuse for one-off (+ endless follow-up) style cleanups. Especially if the new rule doesn't even enforce the new style.

Makes sense that if you're going to to use clang-tidy for a cleanup, you should probably look into it enabling it as a lint-check too so the cleanup does not need to be repeated. I don't have so much of a problem with one-off cleanups or repeated cleanups, though.

Conditional code review ACK 313241d757c022d978d87e1abd9ab4bb68d45986 if it is ok to use c++ syntax in the bitcoinconsensus.h file

Code review ACK 6edd9716e3980b2e0c3894fc48672c7ae15dd4f2. Only changes since last review were suggested cleanups

<!--cf906140f33d8803c4a75a2196329ecb-->

🐙 This pull request conflicts with the target branch and needs rebase.

_{Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".}