tidy: add modernize-use-using

fanquake commented at 10:19 am on July 25, 2022: member

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.

aureleoules commented at 10:53 am on July 25, 2022: member

Concept ACK

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

DrahtBot commented at 6:27 pm on July 25, 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:

#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.

fanquake force-pushed on Jul 29, 2022

fanquake force-pushed on Aug 1, 2022

fanquake commented at 9:24 am on August 1, 2022: member

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

Added some changes to header files.

DrahtBot added the label Needs rebase on Aug 5, 2022

fanquake force-pushed on Aug 15, 2022

DrahtBot removed the label Needs rebase on Aug 15, 2022

in src/test/fuzz/crypto_diff_fuzz_chacha20.cpp:46 in f5284d39a2 outdated

42@@ -43,10 +43,10 @@ typedef unsigned char u8;
43 /* ------------------------------------------------------------------------- */
44 /* Data structures */
45 
46-typedef struct
47+using ECRYPT_ctx = struct

ryanofsky commented at 6:53 pm on August 18, 2022:

In commit “refactor: use “using” over “typedef”” (f5284d39a2b78bb07655c397386b9741495b6d41)

This one is a little weird because it was a C-style struct typedef. Should probably just be updated struct ECRYPT_ctx {

fanquake commented at 7:54 am on August 19, 2022:

Applied this suggestion.

ryanofsky commented at 6:54 pm on August 18, 2022: contributor

Code review ACK d1885b434f3612ac1efe926e230a440bd7eed366

ryanofsky approved

fanquake force-pushed on Aug 19, 2022

fanquake commented at 3:33 pm on August 19, 2022: member

Rebased for #25707.

fanquake requested review from aureleoules on Aug 19, 2022

aureleoules commented at 10:31 pm on August 19, 2022: member

I believe you can change these as well.

  0diff --git a/src/blockencodings.h b/src/blockencodings.h
  1index 67c4e5715..700ebb252 100644
  2--- a/src/blockencodings.h
  3+++ b/src/blockencodings.h
  4@@ -74,14 +74,13 @@ struct PrefilledTransaction {
  5     SERIALIZE_METHODS(PrefilledTransaction, obj) { READWRITE(COMPACTSIZE(obj.index), Using<TransactionCompression>(obj.tx)); }
  6 };
  7 
  8-typedef enum ReadStatus_t
  9-{
 10+using ReadStatus = enum ReadStatus_t {
 11     READ_STATUS_OK,
 12     READ_STATUS_INVALID, // Invalid object, peer is sending bogus crap
 13     READ_STATUS_FAILED, // Failed to process object
 14     READ_STATUS_CHECKBLOCK_FAILED, // Used only by FillBlock to indicate a
 15                                    // failure in CheckBlock.
 16-} ReadStatus;
 17+};
 18 
 19 class CBlockHeaderAndShortTxIDs {
 20 private:
 21diff --git a/src/protocol.h b/src/protocol.h
 22index b85dc0d82..af105f90a 100644
 23--- a/src/protocol.h
 24+++ b/src/protocol.h
 25@@ -33,7 +33,7 @@ public:
 26     static constexpr size_t MESSAGE_SIZE_OFFSET = MESSAGE_START_SIZE + COMMAND_SIZE;
 27     static constexpr size_t CHECKSUM_OFFSET = MESSAGE_SIZE_OFFSET + MESSAGE_SIZE_SIZE;
 28     static constexpr size_t HEADER_SIZE = MESSAGE_START_SIZE + COMMAND_SIZE + MESSAGE_SIZE_SIZE + CHECKSUM_SIZE;
 29-    typedef unsigned char MessageStartChars[MESSAGE_START_SIZE];
 30+    using MessageStartChars = unsigned char[MESSAGE_START_SIZE];
 31 
 32     explicit CMessageHeader() = default;
 33 
 34diff --git a/src/rpc/server.h b/src/rpc/server.h
 35index 01e855605..31fd72ce3 100644
 36--- a/src/rpc/server.h
 37+++ b/src/rpc/server.h
 38@@ -84,7 +84,7 @@ void RPCUnsetTimerInterface(RPCTimerInterface *iface);
 39  */
 40 void RPCRunLater(const std::string& name, std::function<void()> func, int64_t nSeconds);
 41 
 42-typedef RPCHelpMan (*RpcMethodFnType)();
 43+using RpcMethodFnType = RPCHelpMan (*)();
 44 
 45 class CRPCCommand
 46 {
 47diff --git a/src/script/bitcoinconsensus.h b/src/script/bitcoinconsensus.h
 48index 8fea42e4b..fda5e3c5e 100644
 49--- a/src/script/bitcoinconsensus.h
 50+++ b/src/script/bitcoinconsensus.h
 51@@ -33,7 +33,7 @@ extern "C" {
 52 
 53 #define BITCOINCONSENSUS_API_VER 1
 54 
 55-typedef enum bitcoinconsensus_error_t
 56+using bitcoinconsensus_error = enum bitcoinconsensus_error_t
 57 {
 58     bitcoinconsensus_ERR_OK = 0,
 59     bitcoinconsensus_ERR_TX_INDEX,
 60@@ -41,7 +41,8 @@ typedef enum bitcoinconsensus_error_t
 61     bitcoinconsensus_ERR_TX_DESERIALIZE,
 62     bitcoinconsensus_ERR_AMOUNT_REQUIRED,
 63     bitcoinconsensus_ERR_INVALID_FLAGS,
 64-} bitcoinconsensus_error;
 65+};
 66 
 67 /** Script verification flags */
 68 enum
 69diff --git a/src/script/script_error.h b/src/script/script_error.h
 70index 44e68fe0f..2674ca8dd 100644
 71--- a/src/script/script_error.h
 72+++ b/src/script/script_error.h
 73@@ -8,7 +8,7 @@
 74 
 75 #include <string>
 76 
 77-typedef enum ScriptError_t
 78+using ScriptError = enum ScriptError_t
 79 {
 80     SCRIPT_ERR_OK = 0,
 81     SCRIPT_ERR_UNKNOWN_ERROR,
 82@@ -83,7 +83,7 @@ typedef enum ScriptError_t
 83     SCRIPT_ERR_SIG_FINDANDDELETE,
 84 
 85     SCRIPT_ERR_ERROR_COUNT
 86-} ScriptError;
 87+};
 88 
 89 #define SCRIPT_ERR_LAST SCRIPT_ERR_ERROR_COUNT
 90 
 91diff --git a/src/support/events.h b/src/support/events.h
 92index 0a74d0236..24f807ae3 100644
 93--- a/src/support/events.h
 94+++ b/src/support/events.h
 95@@ -19,7 +19,7 @@ struct type##_deleter {\
 96     }\
 97 };\
 98 /* unique ptr typedef */\
 99-typedef std::unique_ptr<struct type, type##_deleter> raii_##type
100+using raii_##type = std::unique_ptr<struct type, type##_deleter>;
101 
102 MAKE_RAII(event_base);
103 MAKE_RAII(event);
104diff --git a/src/support/lockedpool.h b/src/support/lockedpool.h
105index 1c0d0669b..bbbb5d374 100644
106--- a/src/support/lockedpool.h
107+++ b/src/support/lockedpool.h
108@@ -139,7 +139,8 @@ public:
109 
110     /** Callback when allocation succeeds but locking fails.
111      */
112-    typedef bool (*LockingFailed_Callback)();
113+    using LockingFailed_Callback = bool (*)();
114 
115     /** Memory statistics. */
116     struct Stats
117diff --git a/src/tinyformat.h b/src/tinyformat.h
118index bedaa1400..e938ea046 100644
119--- a/src/tinyformat.h
120+++ b/src/tinyformat.h
121@@ -221,7 +221,9 @@ struct is_convertible
122 
123 
124 // Detect when a type is not a wchar_t string
125-template<typename T> struct is_wchar { typedef int tinyformat_wchar_is_not_supported; };
126+template<typename T> struct is_wchar { 
127+    using tinyformat_wchar_is_not_supported = int;
128+};
129 template<> struct is_wchar<wchar_t*> {};
130 template<> struct is_wchar<const wchar_t*> {};
131 template<int n> struct is_wchar<const wchar_t[n]> {};
132@@ -332,7 +334,7 @@ inline void formatValue(std::ostream& out, const char* /*fmtBegin*/,
133 #ifndef TINYFORMAT_ALLOW_WCHAR_STRINGS
134     // Since we don't support printing of wchar_t using "%ls", make it fail at
135     // compile time in preference to printing as a void* at runtime.
136-    typedef typename detail::is_wchar<T>::tinyformat_wchar_is_not_supported DummyType;
137+    using DummyType = typename detail::is_wchar<T>::tinyformat_wchar_is_not_supported;
138     (void) DummyType(); // avoid unused type warning with gcc-4.8
139 #endif
140     // The mess here is to support the %c and %p conversions: if these
141@@ -958,7 +960,7 @@ class FormatList
142 };
143 
144 /// Reference to type-opaque format list for passing to vformat()
145-typedef const FormatList& FormatListRef;
146+using FormatListRef = const FormatList&;
147 
148 
149 namespace detail {

fanquake commented at 3:03 pm on August 20, 2022: member

I believe you can change these as well.

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

fanquake force-pushed on Aug 20, 2022

aureleoules commented at 7:51 am on August 22, 2022: member

ACK 313241d757c022d978d87e1abd9ab4bb68d45986

maflcko commented at 4:41 pm on August 24, 2022: member

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.

ryanofsky commented at 5:23 pm on August 24, 2022: contributor

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.

in src/blockencodings.h:77 in 4f510b0612 outdated

73@@ -74,14 +74,14 @@ struct PrefilledTransaction {
74     SERIALIZE_METHODS(PrefilledTransaction, obj) { READWRITE(COMPACTSIZE(obj.index), Using<TransactionCompression>(obj.tx)); }
75 };
76 
77-typedef enum ReadStatus_t
78+using ReadStatus = enum ReadStatus_t

ryanofsky commented at 5:26 pm on August 24, 2022:

In commit “refactor: use “using” over “typedef”” (4f510b061229f54b5c0d5dcbd91ea28cfca63dae)

Can just say enum ReadStatus { here. ReadStatus_t is never referenced.

in src/script/bitcoinconsensus.h:36 in 313241d757 outdated

32@@ -33,15 +33,15 @@ extern "C" {
33 
34 #define BITCOINCONSENSUS_API_VER 1
35 
36-typedef enum bitcoinconsensus_error_t
37+using bitcoinconsensus_error = enum bitcoinconsensus_error_t

ryanofsky commented at 5:30 pm on August 24, 2022:

In commit “tidy: enable modernize-use-using” (313241d757c022d978d87e1abd9ab4bb68d45986)

Probably should revert this change if this header is supposed to be usable from C code. I assume extern "C" { above means it should be.

ryanofsky approved

ryanofsky commented at 5:35 pm on August 24, 2022: contributor

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

refactor: use "using" over "typedef" dfd911500b

tidy: enable modernize-use-using

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

6edd9716e3

fanquake force-pushed on Aug 31, 2022

in src/script/script_error.h:11 in dfd911500b outdated

 7@@ -8,7 +8,7 @@
 8 
 9 #include <string>
10 
11-typedef enum ScriptError_t
12+using ScriptError = enum ScriptError_t

ryanofsky commented at 5:42 pm on September 1, 2022:

In commit “refactor: use “using” over “typedef”” (dfd911500bdd1a9542f79d1a823b1b66245ad76c)

Would be good for a followup (possible future PR) to drop ScriptError_t in tests and just change this to enum ScriptError

ryanofsky approved

ryanofsky commented at 5:45 pm on September 1, 2022: contributor

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

DrahtBot commented at 5:09 pm on October 20, 2022: contributor

🐙 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”.

DrahtBot added the label Needs rebase on Oct 20, 2022

fanquake closed this on Dec 5, 2022

fanquake deleted the branch on Nov 6, 2023

tidy: add modernize-use-using #25695

Conflicts