From b0a5ce7743b4f0288bdb06a725033d1045a280ca Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 18 May 2020 04:19:50 +0300 Subject: [PATCH] Enable clang-tidy for programs and utils --- .clang-tidy | 14 +++++++------- programs/CMakeLists.txt | 4 ++++ programs/odbc-bridge/ODBCBlockOutputStream.cpp | 4 ++-- src/Common/tests/cow_columns.cpp | 8 ++++---- src/Common/tests/cow_compositions.cpp | 8 ++++---- utils/CMakeLists.txt | 4 ++++ utils/convert-month-partitioned-parts/main.cpp | 2 +- utils/iotest/iotest.cpp | 8 ++++---- utils/iotest/iotest_aio.cpp | 18 +++++++----------- utils/iotest/iotest_nonblock.cpp | 6 +++--- utils/test-data-generator/CMakeLists.txt | 3 +++ .../main.cpp | 4 ++-- utils/zookeeper-cli/zookeeper-cli.cpp | 8 +++----- 13 files changed, 48 insertions(+), 43 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 3c222fbf8d..b0971418e0 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -9,7 +9,7 @@ Checks: '-*, misc-unused-alias-decls, misc-unused-parameters, misc-unused-using-decls, - + modernize-avoid-bind, modernize-loop-convert, modernize-make-shared, @@ -33,7 +33,7 @@ Checks: '-*, performance-no-automatic-move, performance-trivially-destructible, performance-unnecessary-copy-initialization, - + readability-avoid-const-params-in-decls, readability-const-return-type, readability-container-size-empty, @@ -58,7 +58,7 @@ Checks: '-*, readability-simplify-boolean-expr, readability-inconsistent-declaration-parameter-name, readability-identifier-naming, - + bugprone-undelegated-constructor, bugprone-argument-comment, bugprone-bad-signal-to-kill-thread, @@ -102,7 +102,7 @@ Checks: '-*, bugprone-unused-return-value, bugprone-use-after-move, bugprone-virtual-near-miss, - + cert-dcl21-cpp, cert-dcl50-cpp, cert-env33-c, @@ -112,7 +112,7 @@ Checks: '-*, cert-mem57-cpp, cert-msc50-cpp, cert-oop58-cpp, - + google-build-explicit-make-pair, google-build-namespaces, google-default-arguments, @@ -121,9 +121,9 @@ Checks: '-*, google-readability-avoid-underscore-in-googletest-name, google-runtime-int, google-runtime-operator, - + hicpp-exception-baseclass, - + clang-analyzer-core.CallAndMessage, clang-analyzer-core.DivideZero, clang-analyzer-core.NonNullParamChecker, diff --git a/programs/CMakeLists.txt b/programs/CMakeLists.txt index 7cbe2e7a2a..a3d3188653 100644 --- a/programs/CMakeLists.txt +++ b/programs/CMakeLists.txt @@ -1,3 +1,7 @@ +if (USE_CLANG_TIDY) + set (CMAKE_CXX_CLANG_TIDY "${CLANG_TIDY_PATH}") +endif () + # 'clickhouse' binary is a multi purpose tool, # that contain multiple execution modes (client, server, etc.) # each of them is built and linked as a separate library, defined below. diff --git a/programs/odbc-bridge/ODBCBlockOutputStream.cpp b/programs/odbc-bridge/ODBCBlockOutputStream.cpp index c259780523..ab24c008e4 100644 --- a/programs/odbc-bridge/ODBCBlockOutputStream.cpp +++ b/programs/odbc-bridge/ODBCBlockOutputStream.cpp @@ -24,8 +24,8 @@ namespace query.table_id.table_name = table_name; query.columns = std::make_shared(','); query.children.push_back(query.columns); - for (size_t i = 0; i < columns.size(); ++i) - query.columns->children.emplace_back(std::make_shared(columns[i].name)); + for (const auto & column : columns) + query.columns->children.emplace_back(std::make_shared(column.name)); std::stringstream ss; IAST::FormatSettings settings(ss, true); diff --git a/src/Common/tests/cow_columns.cpp b/src/Common/tests/cow_columns.cpp index fa84fc9ebc..404b478f5a 100644 --- a/src/Common/tests/cow_columns.cpp +++ b/src/Common/tests/cow_columns.cpp @@ -56,8 +56,8 @@ int main(int, char **) MutableColumnPtr mut = IColumn::mutate(std::move(y)); mut->set(2); - std::cerr << "refcounts: " << x->use_count() << ", " << y->use_count() << ", " << mut->use_count() << "\n"; - std::cerr << "addresses: " << x.get() << ", " << y.get() << ", " << mut.get() << "\n"; + std::cerr << "refcounts: " << x->use_count() << ", " << mut->use_count() << "\n"; + std::cerr << "addresses: " << x.get() << ", " << mut.get() << "\n"; y = std::move(mut); } @@ -75,8 +75,8 @@ int main(int, char **) MutableColumnPtr mut = IColumn::mutate(std::move(y)); mut->set(3); - std::cerr << "refcounts: " << x->use_count() << ", " << y->use_count() << ", " << mut->use_count() << "\n"; - std::cerr << "addresses: " << x.get() << ", " << y.get() << ", " << mut.get() << "\n"; + std::cerr << "refcounts: " << x->use_count() << ", " << mut->use_count() << "\n"; + std::cerr << "addresses: " << x.get() << ", " << mut.get() << "\n"; y = std::move(mut); } diff --git a/src/Common/tests/cow_compositions.cpp b/src/Common/tests/cow_compositions.cpp index be33f39249..74369e8630 100644 --- a/src/Common/tests/cow_compositions.cpp +++ b/src/Common/tests/cow_compositions.cpp @@ -75,8 +75,8 @@ int main(int, char **) MutableColumnPtr mut = IColumn::mutate(std::move(y)); mut->set(2); - std::cerr << "refcounts: " << x->use_count() << ", " << y->use_count() << ", " << mut->use_count() << "\n"; - std::cerr << "addresses: " << x.get() << ", " << y.get() << ", " << mut.get() << "\n"; + std::cerr << "refcounts: " << x->use_count() << ", " << mut->use_count() << "\n"; + std::cerr << "addresses: " << x.get() << ", " << mut.get() << "\n"; y = std::move(mut); } @@ -94,8 +94,8 @@ int main(int, char **) MutableColumnPtr mut = IColumn::mutate(std::move(y)); mut->set(3); - std::cerr << "refcounts: " << x->use_count() << ", " << y->use_count() << ", " << mut->use_count() << "\n"; - std::cerr << "addresses: " << x.get() << ", " << y.get() << ", " << mut.get() << "\n"; + std::cerr << "refcounts: " << x->use_count() << ", " << mut->use_count() << "\n"; + std::cerr << "addresses: " << x.get() << ", " << ", " << mut.get() << "\n"; y = std::move(mut); } diff --git a/utils/CMakeLists.txt b/utils/CMakeLists.txt index c8531bd63a..94042ea409 100644 --- a/utils/CMakeLists.txt +++ b/utils/CMakeLists.txt @@ -1,3 +1,7 @@ +if (USE_CLANG_TIDY) + set (CMAKE_CXX_CLANG_TIDY "${CLANG_TIDY_PATH}") +endif () + if(MAKE_STATIC_LIBRARIES) set(MAX_LINKER_MEMORY 3500) else() diff --git a/utils/convert-month-partitioned-parts/main.cpp b/utils/convert-month-partitioned-parts/main.cpp index 51ea87d35b..af8e221a10 100644 --- a/utils/convert-month-partitioned-parts/main.cpp +++ b/utils/convert-month-partitioned-parts/main.cpp @@ -30,7 +30,7 @@ void run(String part_path, String date_column, String dest_path) { std::shared_ptr disk = std::make_shared("local", "/", 0); auto old_part_path = Poco::Path::forDirectory(part_path); - String old_part_name = old_part_path.directory(old_part_path.depth() - 1); + const String & old_part_name = old_part_path.directory(old_part_path.depth() - 1); String old_part_path_str = old_part_path.toString(); auto part_info = MergeTreePartInfo::fromPartName(old_part_name, MergeTreeDataFormatVersion(0)); diff --git a/utils/iotest/iotest.cpp b/utils/iotest/iotest.cpp index ed846e3d46..e578a539bc 100644 --- a/utils/iotest/iotest.cpp +++ b/utils/iotest/iotest.cpp @@ -59,9 +59,9 @@ void thread(int fd, int mode, size_t min_offset, size_t max_offset, size_t block for (size_t i = 0; i < count; ++i) { - long rand_result1 = rng(); - long rand_result2 = rng(); - long rand_result3 = rng(); + uint64_t rand_result1 = rng(); + uint64_t rand_result2 = rng(); + uint64_t rand_result3 = rng(); size_t rand_result = rand_result1 ^ (rand_result2 << 22) ^ (rand_result3 << 43); size_t offset; @@ -152,7 +152,7 @@ int mainImpl(int argc, char ** argv) Stopwatch watch; for (size_t i = 0; i < threads; ++i) - pool.scheduleOrThrowOnError(std::bind(thread, fd, mode, min_offset, max_offset, block_size, count)); + pool.scheduleOrThrowOnError([=]{ thread(fd, mode, min_offset, max_offset, block_size, count); }); pool.wait(); fsync(fd); diff --git a/utils/iotest/iotest_aio.cpp b/utils/iotest/iotest_aio.cpp index c0945fbe1e..24508c1dd9 100644 --- a/utils/iotest/iotest_aio.cpp +++ b/utils/iotest/iotest_aio.cpp @@ -13,6 +13,8 @@ int main(int, char **) { return 0; } #include #include #include +#include +#include #include #include #include @@ -52,10 +54,7 @@ void thread(int fd, int mode, size_t min_offset, size_t max_offset, size_t block for (size_t i = 0; i < buffers_count; ++i) buffers[i] = Memory<>(block_size, sysconf(_SC_PAGESIZE)); - drand48_data rand_data; - timespec times; - clock_gettime(CLOCK_THREAD_CPUTIME_ID, ×); - srand48_r(times.tv_nsec, &rand_data); + pcg64_fast rng(randomSeed()); size_t in_progress = 0; size_t blocks_sent = 0; @@ -82,12 +81,9 @@ void thread(int fd, int mode, size_t min_offset, size_t max_offset, size_t block char * buf = buffers[i].data(); - long rand_result1 = 0; - long rand_result2 = 0; - long rand_result3 = 0; - lrand48_r(&rand_data, &rand_result1); - lrand48_r(&rand_data, &rand_result2); - lrand48_r(&rand_data, &rand_result3); + uint64_t rand_result1 = rng(); + uint64_t rand_result2 = rng(); + uint64_t rand_result3 = rng(); size_t rand_result = rand_result1 ^ (rand_result2 << 22) ^ (rand_result3 << 43); size_t offset = min_offset + rand_result % ((max_offset - min_offset) / block_size) * block_size; @@ -172,7 +168,7 @@ int mainImpl(int argc, char ** argv) Stopwatch watch; for (size_t i = 0; i < threads_count; ++i) - pool.scheduleOrThrowOnError(std::bind(thread, fd, mode, min_offset, max_offset, block_size, buffers_count, count)); + pool.scheduleOrThrowOnError([=]{ thread(fd, mode, min_offset, max_offset, block_size, buffers_count, count); }); pool.wait(); watch.stop(); diff --git a/utils/iotest/iotest_nonblock.cpp b/utils/iotest/iotest_nonblock.cpp index 9317e7ed47..524d6298da 100644 --- a/utils/iotest/iotest_nonblock.cpp +++ b/utils/iotest/iotest_nonblock.cpp @@ -113,9 +113,9 @@ int mainImpl(int argc, char ** argv) polls[i].revents = 0; ++ops; - long rand_result1 = rng(); - long rand_result2 = rng(); - long rand_result3 = rng(); + uint64_t rand_result1 = rng(); + uint64_t rand_result2 = rng(); + uint64_t rand_result3 = rng(); size_t rand_result = rand_result1 ^ (rand_result2 << 22) ^ (rand_result3 << 43); size_t offset; diff --git a/utils/test-data-generator/CMakeLists.txt b/utils/test-data-generator/CMakeLists.txt index d8a2111cf0..3a94358e86 100644 --- a/utils/test-data-generator/CMakeLists.txt +++ b/utils/test-data-generator/CMakeLists.txt @@ -1,3 +1,6 @@ +# Disable clang-tidy for protobuf generated files +set (CMAKE_CXX_CLANG_TIDY "") + add_compile_options(-Wno-zero-as-null-pointer-constant -Wno-array-bounds) # Protobuf generated files if (USE_PROTOBUF) diff --git a/utils/zookeeper-adjust-block-numbers-to-parts/main.cpp b/utils/zookeeper-adjust-block-numbers-to-parts/main.cpp index 91431c0164..a896129f91 100644 --- a/utils/zookeeper-adjust-block-numbers-to-parts/main.cpp +++ b/utils/zookeeper-adjust-block-numbers-to-parts/main.cpp @@ -102,7 +102,7 @@ std::unordered_map getPartitionsNeedAdjustingBlockNumbers( std::cout << "Shard: " << shard << std::endl; std::vector use_tables = tables.empty() ? getAllTables(zk, root, shard) : removeNotExistingTables(zk, root, shard, tables); - for (auto table : use_tables) + for (const auto & table : use_tables) { std::cout << "\tTable: " << table << std::endl; std::string table_path = root + "/" + shard + "/" + table; @@ -121,7 +121,7 @@ std::unordered_map getPartitionsNeedAdjustingBlockNumbers( continue; } - for (auto partition : partitions) + for (const auto & partition : partitions) { try { diff --git a/utils/zookeeper-cli/zookeeper-cli.cpp b/utils/zookeeper-cli/zookeeper-cli.cpp index 40755fc016..0a503e7725 100644 --- a/utils/zookeeper-cli/zookeeper-cli.cpp +++ b/utils/zookeeper-cli/zookeeper-cli.cpp @@ -97,10 +97,8 @@ int main(int argc, char ** argv) bool watch = w == "w"; zkutil::EventPtr event = watch ? std::make_shared() : nullptr; std::vector v = zk.getChildren(path, nullptr, event); - for (size_t i = 0; i < v.size(); ++i) - { - std::cout << v[i] << std::endl; - } + for (const auto & child : v) + std::cout << child << std::endl; if (watch) waitForWatch(event); } @@ -193,7 +191,7 @@ int main(int argc, char ** argv) zk.set(path, data, version, &stat); printStat(stat); } - else if (cmd != "") + else if (!cmd.empty()) { std::cout << "commands:\n"; std::cout << " q\n";