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: contributor

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: contributor

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: contributor

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

bitcoin locked this on Nov 5, 2024

tidy: add modernize-use-using #25695

Conflicts