Merge pull request #618 from ethereum/clang-tidy

Adjust clang-tidy config and fix warnings
ethereum · Dec 28, 2021 · 4b92380 · 4b92380
2 parents 13fc12a + b970535
commit 4b92380
Show file tree

Hide file tree

Showing 13 changed files with 142 additions and 112 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -1,11 +1,13 @@
 Checks: >
   bugprone-*,
+  -bugprone-easily-swappable-parameters,
   cert-*,
   -cert-err58-cpp,
   clang-analyzer-*,
   cppcoreguidelines-*,
   -cppcoreguidelines-avoid-c-arrays,
   -cppcoreguidelines-avoid-magic-numbers,
+  -cppcoreguidelines-avoid-non-const-global-variables,
   -cppcoreguidelines-non-private-member-variables-in-classes,
   -cppcoreguidelines-owning-memory,
   -cppcoreguidelines-pro-bounds-array-to-pointer-decay,
@@ -30,14 +32,18 @@ Checks: >
   -misc-non-private-member-variables-in-classes,
   modernize-*,
   -modernize-avoid-c-arrays,
+  -modernize-use-nodiscard,
   -modernize-use-trailing-return-type,
   performance-*,
   portability-*,
   readability-*,
   -readability-braces-around-statements,
   -readability-else-after-return,
+  -readability-function-cognitive-complexity,
+  -readability-identifier-length,
   -readability-magic-numbers,
   -readability-named-parameter,
+  -readability-qualified-auto,
   -readability-uppercase-literal-suffix,
 
 WarningsAsErrors: '*'

diff --git a/circle.yml b/circle.yml
@@ -137,6 +137,8 @@ jobs:
 
   build-clang-coverage:
     executor: linux-clang-latest
+    environment:
+      CMAKE_OPTIONS: -DCMAKE_CXX_CLANG_TIDY=clang-tidy -DCMAKE_C_CLANG_TIDY=clang-tidy
     steps:
       - checkout
       - build:

diff --git a/examples/example.c b/examples/example.c
@@ -25,7 +25,7 @@ int main(int argc, char* argv[])
         return EVMC_LOADER_ABI_VERSION_MISMATCH;
 #else
     const char* config_string = (argc > 1) ? argv[1] : "example-vm.so";
-    enum evmc_loader_error_code error_code;
+    enum evmc_loader_error_code error_code = EVMC_LOADER_UNSPECIFIED_ERROR;
     struct evmc_vm* vm = evmc_load_and_configure(config_string, &error_code);
     if (!vm)
     {

diff --git a/examples/example_host.cpp b/examples/example_host.cpp
@@ -30,11 +30,8 @@ struct account
     {
         // Extremely dumb "hash" function.
         evmc::bytes32 ret{};
-        for (std::vector<uint8_t>::size_type i = 0; i != code.size(); i++)
-        {
-            auto v = code[i];
+        for (const auto v : code)
             ret.bytes[v % sizeof(ret.bytes)] ^= v;
-        }
         return ret;
     }
 };

diff --git a/examples/example_precompiles_vm/example_precompiles_vm.cpp b/examples/example_precompiles_vm/example_precompiles_vm.cpp
@@ -47,9 +47,9 @@ static evmc_result not_implemented()
     return result;
 }
 
-static evmc_result execute(evmc_vm*,
-                           const evmc_host_interface*,
-                           evmc_host_context*,
+static evmc_result execute(evmc_vm* /*vm*/,
+                           const evmc_host_interface* /*host*/,
+                           evmc_host_context* /*context*/,
                            enum evmc_revision rev,
                            const evmc_message* msg,
                            const uint8_t* /*code*/,

diff --git a/examples/example_vm/example_vm.cpp b/examples/example_vm/example_vm.cpp
@@ -52,14 +52,14 @@ evmc_capabilities_flagset get_capabilities(evmc_vm* /*instance*/)
 /// VMs are allowed to omit this method implementation.
 enum evmc_set_option_result set_option(evmc_vm* instance, const char* name, const char* value)
 {
-    ExampleVM* vm = static_cast<ExampleVM*>(instance);
+    auto* vm = static_cast<ExampleVM*>(instance);
     if (std::strcmp(name, "verbose") == 0)
     {
         if (value == nullptr)
             return EVMC_SET_OPTION_INVALID_VALUE;
 
         char* end = nullptr;
-        long int v = std::strtol(value, &end, 0);
+        auto v = std::strtol(value, &end, 0);
         if (end == value)  // Parsing the value failed.
             return EVMC_SET_OPTION_INVALID_VALUE;
         if (v > 9 || v < -1)  // Not in the valid range.
@@ -74,7 +74,7 @@ enum evmc_set_option_result set_option(evmc_vm* instance, const char* name, cons
 /// The Example VM stack representation.
 struct Stack
 {
-    evmc_uint256be items[1024];       ///< The array of stack items, uninitialized.
+    evmc_uint256be items[1024] = {};  ///< The array of stack items.
     evmc_uint256be* pointer = items;  ///< The pointer to the currently first empty stack slot.
 
     /// Pops an item from the top of the stack.
@@ -167,7 +167,7 @@ evmc_result execute(evmc_vm* instance,
                     const uint8_t* code,
                     size_t code_size)
 {
-    ExampleVM* vm = static_cast<ExampleVM*>(instance);
+    auto* vm = static_cast<ExampleVM*>(instance);
 
     if (vm->verbose > 0)
         std::puts("execution started\n");
@@ -333,7 +333,7 @@ evmc_result execute(evmc_vm* instance,
 
             evmc_result call_result = host->call(context, &call_msg);
 
-            evmc_uint256be value = to_uint256(call_result.status_code == EVMC_SUCCESS);
+            evmc_uint256be value = to_uint256(call_result.status_code == EVMC_SUCCESS ? 1 : 0);
             stack.push(value);
 
             if (call_output_size > call_result.output_size)

diff --git a/lib/hex/hex.cpp b/lib/hex/hex.cpp
@@ -88,7 +88,7 @@ std::error_code validate_hex(std::string_view hex) noexcept
     {
         uint8_t sink = {};
         uint8_t& operator*() noexcept { return sink; }
-        noop_output_iterator operator++(int) noexcept { return *this; }
+        noop_output_iterator operator++(int) noexcept { return *this; }  // NOLINT(cert-dcl21-cpp)
     };
 
     try

diff --git a/lib/instructions/instruction_metrics.c b/lib/instructions/instruction_metrics.c
@@ -2127,6 +2127,7 @@ const struct evmc_instruction_metrics* evmc_get_instruction_metrics_table(
         return homestead_metrics;
     case EVMC_FRONTIER:
         return frontier_metrics;
+    default:
+        return NULL;
     }
-    return NULL;
 }
diff --git a/lib/instructions/instruction_names.c b/lib/instructions/instruction_names.c
@@ -1580,6 +1580,7 @@ const char* const* evmc_get_instruction_names_table(enum evmc_revision revision)
         return homestead_names;
     case EVMC_FRONTIER:
         return frontier_names;
+    default:
+        return NULL;
     }
-    return NULL;
 }
diff --git a/lib/loader/loader.c b/lib/loader/loader.c
@@ -27,6 +27,7 @@
 #define DLL_HANDLE void*
 #define DLL_OPEN(filename) dlopen(filename, RTLD_LAZY)
 #define DLL_CLOSE(handle) dlclose(handle)
+// NOLINTNEXTLINE(performance-no-int-to-ptr)
 #define DLL_GET_CREATE_FN(handle, name) (evmc_create_fn)(uintptr_t) dlsym(handle, name)
 #define DLL_GET_ERROR_MSG() dlerror()
 #endif
@@ -60,6 +61,7 @@ static
         dest[0] = 0;
         return 1;
     }
+    // NOLINTNEXTLINE(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
     memcpy(dest, src, len);
     dest[len] = 0;
     return 0;
@@ -82,6 +84,7 @@ static enum evmc_loader_error_code set_error(enum evmc_loader_error_code error_c
 {
     va_list args;
     va_start(args, format);
+    // NOLINTNEXTLINE(clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling)
     if (vsnprintf(last_error_msg_buffer, LAST_ERROR_MSG_BUFFER_SIZE, format, args) <
         LAST_ERROR_MSG_BUFFER_SIZE)
         last_error_msg = last_error_msg_buffer;

diff --git a/test/unittests/cpp_test.cpp b/test/unittests/cpp_test.cpp
@@ -21,52 +21,62 @@
 class NullHost : public evmc::Host
 {
 public:
-    bool account_exists(const evmc::address&) const noexcept final { return false; }
+    bool account_exists(const evmc::address& /*addr*/) const noexcept final { return false; }
 
-    evmc::bytes32 get_storage(const evmc::address&, const evmc::bytes32&) const noexcept final
+    evmc::bytes32 get_storage(const evmc::address& /*addr*/,
+                              const evmc::bytes32& /*key*/) const noexcept final
     {
         return {};
     }
 
-    evmc_storage_status set_storage(const evmc::address&,
-                                    const evmc::bytes32&,
-                                    const evmc::bytes32&) noexcept final
+    evmc_storage_status set_storage(const evmc::address& /*addr*/,
+                                    const evmc::bytes32& /*key*/,
+                                    const evmc::bytes32& /*value*/) noexcept final
     {
         return {};
     }
 
-    evmc::uint256be get_balance(const evmc::address&) const noexcept final { return {}; }
+    evmc::uint256be get_balance(const evmc::address& /*addr*/) const noexcept final { return {}; }
 
-    size_t get_code_size(const evmc::address&) const noexcept final { return 0; }
+    size_t get_code_size(const evmc::address& /*addr*/) const noexcept final { return 0; }
 
-    evmc::bytes32 get_code_hash(const evmc::address&) const noexcept final { return {}; }
+    evmc::bytes32 get_code_hash(const evmc::address& /*addr*/) const noexcept final { return {}; }
 
-    size_t copy_code(const evmc::address&, size_t, uint8_t*, size_t) const noexcept final
+    size_t copy_code(const evmc::address& /*addr*/,
+                     size_t /*code_offset*/,
+                     uint8_t* /*buffer_data*/,
+                     size_t /*buffer_size*/) const noexcept final
     {
         return 0;
     }
 
-    void selfdestruct(const evmc::address&, const evmc::address&) noexcept final {}
+    void selfdestruct(const evmc::address& /*addr*/,
+                      const evmc::address& /*beneficiary*/) noexcept final
+    {}
 
-    evmc::result call(const evmc_message&) noexcept final { return evmc::result{evmc_result{}}; }
+    evmc::result call(const evmc_message& /*msg*/) noexcept final
+    {
+        return evmc::result{evmc_result{}};
+    }
 
     evmc_tx_context get_tx_context() const noexcept final { return {}; }
 
-    evmc::bytes32 get_block_hash(int64_t) const noexcept final { return {}; }
+    evmc::bytes32 get_block_hash(int64_t /*block_number*/) const noexcept final { return {}; }
 
-    void emit_log(const evmc::address&,
-                  const uint8_t*,
-                  size_t,
-                  const evmc::bytes32[],
-                  size_t) noexcept final
+    void emit_log(const evmc::address& /*addr*/,
+                  const uint8_t* /*data*/,
+                  size_t /*data_size*/,
+                  const evmc::bytes32 /*topics*/[],
+                  size_t /*num_topics*/) noexcept final
     {}
 
-    evmc_access_status access_account(const evmc::address&) noexcept final
+    evmc_access_status access_account(const evmc::address& /*addr*/) noexcept final
     {
         return EVMC_ACCESS_COLD;
     }
 
-    evmc_access_status access_storage(const evmc::address&, const evmc::bytes32&) noexcept final
+    evmc_access_status access_storage(const evmc::address& /*addr*/,
+                                      const evmc::bytes32& /*key*/) noexcept final
     {
         return EVMC_ACCESS_COLD;
     }
@@ -532,12 +542,12 @@ TEST(cpp, vm_move)
         EXPECT_TRUE(vm1);
         auto vm2 = std::move(vm1);
         EXPECT_TRUE(vm2);
-        EXPECT_FALSE(vm1);  // NOLINT
-        EXPECT_EQ(vm1.get_raw_pointer(), nullptr);
+        EXPECT_FALSE(vm1);                          // NOLINT
+        EXPECT_EQ(vm1.get_raw_pointer(), nullptr);  // NOLINT
         auto vm3 = std::move(vm2);
         EXPECT_TRUE(vm3);
-        EXPECT_FALSE(vm2);  // NOLINT
-        EXPECT_EQ(vm2.get_raw_pointer(), nullptr);
+        EXPECT_FALSE(vm2);                          // NOLINT
+        EXPECT_EQ(vm2.get_raw_pointer(), nullptr);  // NOLINT
         EXPECT_FALSE(vm1);
         EXPECT_EQ(vm1.get_raw_pointer(), nullptr);
     }
@@ -785,6 +795,7 @@ TEST(cpp, status_code_to_string)
         std::string_view str;
     };
 
+// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
 #define TEST_CASE(STATUS_CODE) \
     TestCase { STATUS_CODE, #STATUS_CODE }
     constexpr TestCase test_cases[]{
@@ -835,6 +846,7 @@ TEST(cpp, revision_to_string)
         std::string_view str;
     };
 
+// NOLINTNEXTLINE(cppcoreguidelines-macro-usage)
 #define TEST_CASE(STATUS_CODE) \
     TestCase { STATUS_CODE, #STATUS_CODE }
     constexpr TestCase test_cases[]{
@@ -883,7 +895,7 @@ TEST(cpp, revision_to_string)
 
 
 #ifdef __GNUC__
-extern "C" [[gnu::weak]] void __ubsan_handle_builtin_unreachable(void*);
+extern "C" [[gnu::weak]] void __ubsan_handle_builtin_unreachable(void*);  // NOLINT
 #endif
 
 static bool has_ubsan() noexcept

diff --git a/test/unittests/loader_test.cpp b/test/unittests/loader_test.cpp
@@ -54,9 +54,11 @@ class loader : public ::testing::Test
         evmc_test_create_fn = fn;
     }
 
-    static void destroy(evmc_vm*) noexcept { ++destroy_count; }
+    static void destroy(evmc_vm* /*vm*/) noexcept { ++destroy_count; }
 
-    static evmc_set_option_result set_option(evmc_vm*, const char* name, const char* value) noexcept
+    static evmc_set_option_result set_option(evmc_vm* /*vm*/,
+                                             const char* name,
+                                             const char* value) noexcept
     {
         recorded_options.push_back({name, value});  // NOLINT