Merge pull request ClickHouse#61150 from rschu1ze/clang-tidy-list

Some clang-tidy fixes

.clang-tidy

@@ -5,128 +5,127 @@
 # a) the new check is not controversial (this includes many checks in readability-* and google-*) or
 # b) too noisy (checks with > 100 new warnings are considered noisy, this includes e.g. cppcoreguidelines-*).
 
-# TODO: Once clang(-tidy) 17 is the minimum, we can convert this list to YAML
-# See https://releases.llvm.org/17.0.1/tools/clang/tools/extra/docs/ReleaseNotes.html#improvements-to-clang-tidy
-
 # TODO Let clang-tidy check headers in further directories
 #      --> HeaderFilterRegex: '^.*/(src|base|programs|utils)/.*(h|hpp)$'
 HeaderFilterRegex: '^.*/(base|programs|utils)/.*(h|hpp)$'
 
-Checks: '*,
-    -abseil-*,
-
-    -altera-*,
-
-    -android-*,
-
-    -bugprone-assignment-in-if-condition,
-    -bugprone-branch-clone,
-    -bugprone-easily-swappable-parameters,
-    -bugprone-exception-escape,
-    -bugprone-implicit-widening-of-multiplication-result,
-    -bugprone-narrowing-conversions,
-    -bugprone-not-null-terminated-result,
-    -bugprone-reserved-identifier, # useful but too slow, TODO retry when https://reviews.llvm.org/rG1c282052624f9d0bd273bde0b47b30c96699c6c7 is merged
-    -bugprone-unchecked-optional-access,
-
-    -cert-dcl16-c,
-    -cert-dcl37-c,
-    -cert-dcl51-cpp,
-    -cert-err58-cpp,
-    -cert-msc32-c,
-    -cert-msc51-cpp,
-    -cert-oop54-cpp,
-    -cert-oop57-cpp,
-
-    -clang-analyzer-unix.Malloc,
-
-    -cppcoreguidelines-*, # impractical in a codebase as large as ClickHouse, also slow
-
-    -darwin-*,
-
-    -fuchsia-*,
-
-    -google-build-using-namespace,
-    -google-readability-braces-around-statements,
-    -google-readability-casting,
-    -google-readability-function-size,
-    -google-readability-namespace-comments,
-    -google-readability-todo,
-
-    -hicpp-avoid-c-arrays,
-    -hicpp-avoid-goto,
-    -hicpp-braces-around-statements,
-    -hicpp-explicit-conversions,
-    -hicpp-function-size,
-    -hicpp-member-init,
-    -hicpp-move-const-arg,
-    -hicpp-multiway-paths-covered,
-    -hicpp-named-parameter,
-    -hicpp-no-array-decay,
-    -hicpp-no-assembler,
-    -hicpp-no-malloc,
-    -hicpp-signed-bitwise,
-    -hicpp-special-member-functions,
-    -hicpp-uppercase-literal-suffix,
-    -hicpp-use-auto,
-    -hicpp-use-emplace,
-    -hicpp-vararg,
-
-    -linuxkernel-*,
-
-    -llvm-*,
-
-    -llvmlibc-*,
-
-    -openmp-*,
-
-    -misc-const-correctness,
-    -misc-include-cleaner, # useful but far too many occurrences
-    -misc-no-recursion,
-    -misc-non-private-member-variables-in-classes,
-    -misc-confusable-identifiers, # useful but slooow
-    -misc-use-anonymous-namespace,
-
-    -modernize-avoid-c-arrays,
-    -modernize-concat-nested-namespaces,
-    -modernize-macro-to-enum,
-    -modernize-pass-by-value,
-    -modernize-return-braced-init-list,
-    -modernize-use-auto,
-    -modernize-use-default-member-init,
-    -modernize-use-emplace,
-    -modernize-use-nodiscard,
-    -modernize-use-override,
-    -modernize-use-trailing-return-type,
-
-    -performance-inefficient-string-concatenation,
-    -performance-no-int-to-ptr,
-    -performance-avoid-endl,
-    -performance-unnecessary-value-param,
-
-    -portability-simd-intrinsics,
-
-    -readability-avoid-unconditional-preprocessor-if,
-    -readability-braces-around-statements,
-    -readability-convert-member-functions-to-static,
-    -readability-else-after-return,
-    -readability-function-cognitive-complexity,
-    -readability-function-size,
-    -readability-identifier-length,
-    -readability-identifier-naming, # useful but too slow
-    -readability-implicit-bool-conversion,
-    -readability-isolate-declaration,
-    -readability-magic-numbers,
-    -readability-named-parameter,
-    -readability-redundant-declaration,
-    -readability-simplify-boolean-expr,
-    -readability-static-accessed-through-instance,
-    -readability-suspicious-call-argument,
-    -readability-uppercase-literal-suffix,
-    -readability-use-anyofallof,
-
-    -zircon-*,
-'
+Checks: [
+  '*',
+
+  '-abseil-*',
+
+  '-altera-*',
+
+  '-android-*',
+
+  '-bugprone-assignment-in-if-condition',
+  '-bugprone-branch-clone',
+  '-bugprone-easily-swappable-parameters',
+  '-bugprone-exception-escape',
+  '-bugprone-implicit-widening-of-multiplication-result',
+  '-bugprone-narrowing-conversions',
+  '-bugprone-not-null-terminated-result',
+  '-bugprone-reserved-identifier', # useful but too slow, TODO retry when https://reviews.llvm.org/rG1c282052624f9d0bd273bde0b47b30c96699c6c7 is merged
+  '-bugprone-unchecked-optional-access',
+
+  '-cert-dcl16-c',
+  '-cert-dcl37-c',
+  '-cert-dcl51-cpp',
+  '-cert-err58-cpp',
+  '-cert-msc32-c',
+  '-cert-msc51-cpp',
+  '-cert-oop54-cpp',
+  '-cert-oop57-cpp',
+
+  '-clang-analyzer-unix.Malloc',
+
+  '-cppcoreguidelines-*', # impractical in a codebase as large as ClickHouse, also slow
+
+  '-darwin-*',
+
+  '-fuchsia-*',
+
+  '-google-build-using-namespace',
+  '-google-readability-braces-around-statements',
+  '-google-readability-casting',
+  '-google-readability-function-size',
+  '-google-readability-namespace-comments',
+  '-google-readability-todo',
+
+  '-hicpp-avoid-c-arrays',
+  '-hicpp-avoid-goto',
+  '-hicpp-braces-around-statements',
+  '-hicpp-explicit-conversions',
+  '-hicpp-function-size',
+  '-hicpp-member-init',
+  '-hicpp-move-const-arg',
+  '-hicpp-multiway-paths-covered',
+  '-hicpp-named-parameter',
+  '-hicpp-no-array-decay',
+  '-hicpp-no-assembler',
+  '-hicpp-no-malloc',
+  '-hicpp-signed-bitwise',
+  '-hicpp-special-member-functions',
+  '-hicpp-uppercase-literal-suffix',
+  '-hicpp-use-auto',
+  '-hicpp-use-emplace',
+  '-hicpp-vararg',
+
+  '-linuxkernel-*',
+
+  '-llvm-*',
+
+  '-llvmlibc-*',
+
+  '-openmp-*',
+
+  '-misc-const-correctness',
+  '-misc-include-cleaner', # useful but far too many occurrences
+  '-misc-no-recursion',
+  '-misc-non-private-member-variables-in-classes',
+  '-misc-confusable-identifiers', # useful but slooo
+  '-misc-use-anonymous-namespace',
+
+  '-modernize-avoid-c-arrays',
+  '-modernize-concat-nested-namespaces',
+  '-modernize-macro-to-enum',
+  '-modernize-pass-by-value',
+  '-modernize-return-braced-init-list',
+  '-modernize-use-auto',
+  '-modernize-use-default-member-init',
+  '-modernize-use-emplace',
+  '-modernize-use-nodiscard',
+  '-modernize-use-override',
+  '-modernize-use-trailing-return-type',
+
+  '-performance-inefficient-string-concatenation',
+  '-performance-no-int-to-ptr',
+  '-performance-avoid-endl',
+  '-performance-unnecessary-value-param',
+
+  '-portability-simd-intrinsics',
+
+  '-readability-avoid-unconditional-preprocessor-if',
+  '-readability-braces-around-statements',
+  '-readability-convert-member-functions-to-static',
+  '-readability-else-after-return',
+  '-readability-function-cognitive-complexity',
+  '-readability-function-size',
+  '-readability-identifier-length',
+  '-readability-identifier-naming', # useful but too slow
+  '-readability-implicit-bool-conversion',
+  '-readability-isolate-declaration',
+  '-readability-magic-numbers',
+  '-readability-named-parameter',
+  '-readability-redundant-declaration',
+  '-readability-simplify-boolean-expr',
+  '-readability-static-accessed-through-instance',
+  '-readability-suspicious-call-argument',
+  '-readability-uppercase-literal-suffix',
+  '-readability-use-anyofallof',
+
+  '-zircon-*'
+]
 
 WarningsAsErrors: '*'
 

base/base/coverage.cpp

@@ -1,7 +1,7 @@
 #include "coverage.h"
 #include <sys/mman.h>
 
-#pragma GCC diagnostic ignored "-Wreserved-identifier"
+#pragma clang diagnostic ignored "-Wreserved-identifier"
 
 
 /// WITH_COVERAGE enables the default implementation of code coverage,

base/base/sort.h

@@ -59,8 +59,8 @@ using ComparatorWrapper = Comparator;
 
 #endif
 
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wold-style-cast"
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wold-style-cast"
 
 #include <miniselect/floyd_rivest_select.h>
 
@@ -115,7 +115,7 @@ void partial_sort(RandomIt first, RandomIt middle, RandomIt last)
     ::partial_sort(first, middle, last, comparator());
 }
 
-#pragma GCC diagnostic pop
+#pragma clang diagnostic pop
 
 template <typename RandomIt, typename Compare>
 void sort(RandomIt first, RandomIt last, Compare compare)

contrib/libmetrohash/src/metrohash128.h

@@ -17,6 +17,8 @@
 #ifndef METROHASH_METROHASH_128_H
 #define METROHASH_METROHASH_128_H
 
+// NOLINTBEGIN(readability-avoid-const-params-in-decls)
+
 #include <stdint.h>
 
 class MetroHash128
@@ -68,5 +70,6 @@ class MetroHash128
 void metrohash128_1(const uint8_t * key, uint64_t len, uint32_t seed, uint8_t * out);
 void metrohash128_2(const uint8_t * key, uint64_t len, uint32_t seed, uint8_t * out);
 
+// NOLINTEND(readability-avoid-const-params-in-decls)
 
 #endif // #ifndef METROHASH_METROHASH_128_H

programs/client/Client.cpp

@@ -1392,8 +1392,8 @@ void Client::readArguments(
 }
 
 
-#pragma GCC diagnostic ignored "-Wunused-function"
-#pragma GCC diagnostic ignored "-Wmissing-declarations"
+#pragma clang diagnostic ignored "-Wunused-function"
+#pragma clang diagnostic ignored "-Wmissing-declarations"
 
 int mainEntryClickHouseClient(int argc, char ** argv)
 {

programs/extract-from-config/ExtractFromConfig.cpp

@@ -109,8 +109,8 @@ static std::vector<std::string> extractFromConfig(
     return {configuration->getString(key)};
 }
 
-#pragma GCC diagnostic ignored "-Wunused-function"
-#pragma GCC diagnostic ignored "-Wmissing-declarations"
+#pragma clang diagnostic ignored "-Wunused-function"
+#pragma clang diagnostic ignored "-Wmissing-declarations"
 
 int mainEntryClickHouseExtractFromConfig(int argc, char ** argv)
 {

programs/format/Format.cpp

@@ -70,8 +70,8 @@ void skipSpacesAndComments(const char*& pos, const char* end, bool print_comment
 
 }
 
-#pragma GCC diagnostic ignored "-Wunused-function"
-#pragma GCC diagnostic ignored "-Wmissing-declarations"
+#pragma clang diagnostic ignored "-Wunused-function"
+#pragma clang diagnostic ignored "-Wmissing-declarations"
 
 extern const char * auto_time_zones[];
 

programs/local/LocalServer.cpp

@@ -959,8 +959,8 @@ void LocalServer::readArguments(int argc, char ** argv, Arguments & common_argum
 
 }
 
-#pragma GCC diagnostic ignored "-Wunused-function"
-#pragma GCC diagnostic ignored "-Wmissing-declarations"
+#pragma clang diagnostic ignored "-Wunused-function"
+#pragma clang diagnostic ignored "-Wmissing-declarations"
 
 int mainEntryClickHouseLocal(int argc, char ** argv)
 {

programs/obfuscator/Obfuscator.cpp

@@ -1204,8 +1204,8 @@ class Obfuscator
 
 }
 
-#pragma GCC diagnostic ignored "-Wunused-function"
-#pragma GCC diagnostic ignored "-Wmissing-declarations"
+#pragma clang diagnostic ignored "-Wunused-function"
+#pragma clang diagnostic ignored "-Wmissing-declarations"
 
 int mainEntryClickHouseObfuscator(int argc, char ** argv)
 try

src/Access/AccessControl.h

@@ -133,20 +133,20 @@ class AccessControl : public MultipleAccessStorage
     /// This function also enables custom prefixes to be used.
     void setCustomSettingsPrefixes(const Strings & prefixes);
     void setCustomSettingsPrefixes(const String & comma_separated_prefixes);
-    bool isSettingNameAllowed(const std::string_view name) const;
-    void checkSettingNameIsAllowed(const std::string_view name) const;
+    bool isSettingNameAllowed(std::string_view name) const;
+    void checkSettingNameIsAllowed(std::string_view name) const;
 
     /// Allows implicit user creation without password (by default it's allowed).
     /// In other words, allow 'CREATE USER' queries without 'IDENTIFIED WITH' clause.
-    void setImplicitNoPasswordAllowed(const bool allow_implicit_no_password_);
+    void setImplicitNoPasswordAllowed(bool allow_implicit_no_password_);
     bool isImplicitNoPasswordAllowed() const;
 
     /// Allows users without password (by default it's allowed).
-    void setNoPasswordAllowed(const bool allow_no_password_);
+    void setNoPasswordAllowed(bool allow_no_password_);
     bool isNoPasswordAllowed() const;
 
     /// Allows users with plaintext password (by default it's allowed).
-    void setPlaintextPasswordAllowed(const bool allow_plaintext_password_);
+    void setPlaintextPasswordAllowed(bool allow_plaintext_password_);
     bool isPlaintextPasswordAllowed() const;
 
     /// Default password type when the user does not specify it.

src/Access/IAccessStorage.cpp

@@ -616,7 +616,7 @@ UUID IAccessStorage::generateRandomID()
 }
 
 
-void IAccessStorage::clearConflictsInEntitiesList(std::vector<std::pair<UUID, AccessEntityPtr>> & entities, const LoggerPtr log_)
+void IAccessStorage::clearConflictsInEntitiesList(std::vector<std::pair<UUID, AccessEntityPtr>> & entities, LoggerPtr log_)
 {
     std::unordered_map<UUID, size_t> positions_by_id;
     std::unordered_map<std::string_view, size_t> positions_by_type_and_name[static_cast<size_t>(AccessEntityType::MAX)];

src/Access/IAccessStorage.h

@@ -228,7 +228,7 @@ class IAccessStorage : public boost::noncopyable
     static UUID generateRandomID();
     LoggerPtr getLogger() const;
     static String formatEntityTypeWithName(AccessEntityType type, const String & name) { return AccessEntityTypeInfo::get(type).formatEntityNameWithType(name); }
-    static void clearConflictsInEntitiesList(std::vector<std::pair<UUID, AccessEntityPtr>> & entities, const LoggerPtr log_);
+    static void clearConflictsInEntitiesList(std::vector<std::pair<UUID, AccessEntityPtr>> & entities, LoggerPtr log_);
     [[noreturn]] void throwNotFound(const UUID & id) const;
     [[noreturn]] void throwNotFound(AccessEntityType type, const String & name) const;
     [[noreturn]] static void throwBadCast(const UUID & id, AccessEntityType type, const String & name, AccessEntityType required_type);