clang-tidy, part 2

.clang-tidy

@@ -1,2 +1,63 @@
-Checks: '-*,google-readability-avoid-underscore-in-googletest-name,misc-throw-by-value-catch-by-reference,misc-misplaced-const,misc-unconventional-assign-operator,modernize-avoid-bind,modernize-loop-convert,modernize-make-shared,modernize-make-unique,modernize-raw-string-literal,modernize-redundant-void-arg,modernize-replace-auto-ptr,modernize-replace-random-shuffle,modernize-use-bool-literals,modernize-use-nullptr,modernize-use-using,performance-faster-string-find,performance-for-range-copy,readability-avoid-const-params-in-decls,readability-const-return-type,readability-container-size-empty,readability-convert-member-functions-to-static,readability-delete-null-pointer,readability-deleted-default,readability-make-member-function-const,readability-misplaced-array-index,readability-non-const-parameter,readability-qualified-auto,readability-redundant-access-specifiers,readability-redundant-control-flow,readability-redundant-function-ptr-dereference,readability-redundant-smartptr-get,readability-redundant-string-cstr,readability-redundant-string-init,readability-static-definition-in-anonymous-namespace,readability-string-compare,readability-uniqueptr-delete-release,modernize-use-equals-default,modernize-use-equals-delete,bugprone-undelegated-constructor,readability-redundant-member-init,readability-simplify-subscript-expr,readability-simplify-boolean-expr,readability-inconsistent-declaration-parameter-name'
+Checks: '-*,
+    google-readability-avoid-underscore-in-googletest-name,
+
+    misc-throw-by-value-catch-by-reference,
+    misc-misplaced-const,
+    misc-unconventional-assign-operator,
+
+    modernize-avoid-bind,
+    modernize-loop-convert,
+    modernize-make-shared,
+    modernize-make-unique,
+    modernize-raw-string-literal,
+    modernize-redundant-void-arg,
+    modernize-replace-auto-ptr,
+    modernize-replace-random-shuffle,
+    modernize-use-bool-literals,
+    modernize-use-nullptr,
+    modernize-use-using,
+    modernize-use-equals-default,
+    modernize-use-equals-delete,
+
+    performance-faster-string-find,
+    performance-for-range-copy,
+
+    readability-avoid-const-params-in-decls,
+    readability-const-return-type,
+    readability-container-size-empty,
+    readability-convert-member-functions-to-static,
+    readability-delete-null-pointer,
+    readability-deleted-default,
+    readability-make-member-function-const,
+    readability-misplaced-array-index,
+    readability-non-const-parameter,
+    readability-qualified-auto,
+    readability-redundant-access-specifiers,
+    readability-redundant-control-flow,
+    readability-redundant-function-ptr-dereference,
+    readability-redundant-smartptr-get,
+    readability-redundant-string-cstr,
+    readability-redundant-string-init,
+    readability-static-definition-in-anonymous-namespace,
+    readability-string-compare,
+    readability-uniqueptr-delete-release,
+    readability-redundant-member-init,
+    readability-simplify-subscript-expr,
+    readability-simplify-boolean-expr,
+    readability-inconsistent-declaration-parameter-name,
+
+    bugprone-undelegated-constructor,
+    bugprone-argument-comment,
+    bugprone-bad-signal-to-kill-thread,
+    bugprone-bool-pointer-implicit-conversion,
+    bugprone-copy-constructor-init,
+    bugprone-dangling-handle,
+    bugprone-forward-declaration-namespace,
+    bugprone-fold-init-type,
+    bugprone-inaccurate-erase,
+    bugprone-incorrect-roundings,
+    bugprone-infinite-loop,
+
+    boost-use-to-string,
+'
 WarningsAsErrors: '*'

base/daemon/BaseDaemon.cpp

@@ -362,19 +362,8 @@ void BaseDaemon::reloadConfiguration()
 }
 
 
-BaseDaemon::BaseDaemon()
+namespace
 {
-    checkRequiredInstructions();
-}
-
-
-BaseDaemon::~BaseDaemon()
-{
-    writeSignalIDtoSignalPipe(SignalListener::StopThread);
-    signal_listener_thread.join();
-    signal_pipe.close();
-}
-
 
 enum class InstructionFail
 {
@@ -388,7 +377,7 @@ enum class InstructionFail
     AVX512 = 7
 };
 
-static std::string instructionFailToString(InstructionFail fail)
+std::string instructionFailToString(InstructionFail fail)
 {
     switch (fail)
     {
@@ -413,16 +402,16 @@ static std::string instructionFailToString(InstructionFail fail)
 }
 
 
-static sigjmp_buf jmpbuf;
+sigjmp_buf jmpbuf;
 
-static void sigIllCheckHandler(int sig, siginfo_t * info, void * context)
+void sigIllCheckHandler(int sig, siginfo_t * info, void * context)
 {
     siglongjmp(jmpbuf, 1);
 }
 
 /// Check if necessary sse extensions are available by trying to execute some sse instructions.
 /// If instruction is unavailable, SIGILL will be sent by kernel.
-static void checkRequiredInstructions(volatile InstructionFail & fail)
+void checkRequiredInstructionsImpl(volatile InstructionFail & fail)
 {
 #if __SSE3__
     fail = InstructionFail::SSE3;
@@ -463,8 +452,9 @@ static void checkRequiredInstructions(volatile InstructionFail & fail)
     fail = InstructionFail::NONE;
 }
 
-
-void BaseDaemon::checkRequiredInstructions()
+/// Check SSE and others instructions availability
+/// Calls exit on fail
+void checkRequiredInstructions()
 {
     struct sigaction sa{};
     struct sigaction sa_old{};
@@ -487,7 +477,7 @@ void BaseDaemon::checkRequiredInstructions()
         exit(1);
     }
 
-    ::checkRequiredInstructions(fail);
+    checkRequiredInstructionsImpl(fail);
 
     if (sigaction(signal, &sa_old, nullptr))
     {
@@ -496,6 +486,22 @@ void BaseDaemon::checkRequiredInstructions()
     }
 }
 
+}
+
+
+BaseDaemon::BaseDaemon()
+{
+    checkRequiredInstructions();
+}
+
+
+BaseDaemon::~BaseDaemon()
+{
+    writeSignalIDtoSignalPipe(SignalListener::StopThread);
+    signal_listener_thread.join();
+    signal_pipe.close();
+}
+
 
 void BaseDaemon::terminate()
 {

base/daemon/BaseDaemon.h

@@ -128,7 +128,7 @@ class BaseDaemon : public Poco::Util::ServerApplication, public Loggers
     /// close all process FDs except
     /// 0-2 -- stdin, stdout, stderr
     /// also doesn't close global internal pipes for signal handling
-    void closeFDs();
+    static void closeFDs();
 
 protected:
     /// Возвращает TaskManager приложения
@@ -198,12 +198,6 @@ class BaseDaemon : public Poco::Util::ServerApplication, public Loggers
     std::string config_path;
     DB::ConfigProcessor::LoadedConfig loaded_config;
     Poco::Util::AbstractConfiguration * last_configuration = nullptr;
-
-private:
-
-    /// Check SSE and others instructions availability
-    /// Calls exit on fail
-    void checkRequiredInstructions();
 };
 
 

dbms/programs/client/Client.cpp

@@ -281,7 +281,7 @@ class Client : public Poco::Util::Application
     }
 
     /// Should we celebrate a bit?
-    bool isNewYearMode()
+    static bool isNewYearMode()
     {
         time_t current_time = time(nullptr);
 
@@ -294,7 +294,7 @@ class Client : public Poco::Util::Application
             || (now.month() == 1 && now.day() <= 5);
     }
 
-    bool isChineseNewYearMode(const String & local_tz)
+    static bool isChineseNewYearMode(const String & local_tz)
     {
         /// Days of Dec. 20 in Chinese calendar starting from year 2019 to year 2105
         static constexpr UInt16 chineseNewYearIndicators[]
@@ -1594,7 +1594,7 @@ class Client : public Poco::Util::Application
             std::cout << "Ok." << std::endl;
     }
 
-    void showClientVersion()
+    static void showClientVersion()
     {
         std::cout << DBMS_NAME << " client version " << VERSION_STRING << VERSION_OFFICIAL << "." << std::endl;
     }

dbms/programs/copier/ClusterCopier.cpp

@@ -499,7 +499,10 @@ ASTPtr ClusterCopier::removeAliasColumnsFromCreateQuery(const ASTPtr & query_ast
     return new_query_ast;
 }
 
-std::shared_ptr<ASTCreateQuery> ClusterCopier::rewriteCreateQueryStorage(const ASTPtr & create_query_ast, const DatabaseAndTableName & new_table, const ASTPtr & new_storage_ast)
+
+/// Replaces ENGINE and table name in a create query
+static std::shared_ptr<ASTCreateQuery> rewriteCreateQueryStorage(
+    const ASTPtr & create_query_ast, const DatabaseAndTableName & new_table, const ASTPtr & new_storage_ast)
 {
     const auto & create = create_query_ast->as<ASTCreateQuery &>();
     auto res = std::make_shared<ASTCreateQuery>(create);

dbms/programs/copier/ClusterCopier.h

@@ -88,11 +88,6 @@ class ClusterCopier
     /// Removes MATERIALIZED and ALIAS columns from create table query
     static ASTPtr removeAliasColumnsFromCreateQuery(const ASTPtr &query_ast);
 
-    /// Replaces ENGINE and table name in a create query
-    std::shared_ptr<ASTCreateQuery>
-    rewriteCreateQueryStorage(const ASTPtr & create_query_ast, const DatabaseAndTableName & new_table,
-                              const ASTPtr & new_storage_ast);
-
     bool tryDropPartition(ShardPartition & task_partition,
                           const zkutil::ZooKeeperPtr & zookeeper,
                           const CleanStateClock & clean_state_clock);

dbms/programs/local/LocalServer.cpp

@@ -118,6 +118,20 @@ void LocalServer::tryInitPath()
 }
 
 
+static void attachSystemTables()
+{
+    DatabasePtr system_database = DatabaseCatalog::instance().tryGetDatabase(DatabaseCatalog::SYSTEM_DATABASE);
+    if (!system_database)
+    {
+        /// TODO: add attachTableDelayed into DatabaseMemory to speedup loading
+        system_database = std::make_shared<DatabaseMemory>(DatabaseCatalog::SYSTEM_DATABASE);
+        DatabaseCatalog::instance().attachDatabase(DatabaseCatalog::SYSTEM_DATABASE, system_database);
+    }
+
+    attachSystemTablesLocal(*system_database);
+}
+
+
 int LocalServer::main(const std::vector<std::string> & /*args*/)
 try
 {
@@ -248,20 +262,6 @@ std::string LocalServer::getInitialCreateTableQuery()
 }
 
 
-void LocalServer::attachSystemTables()
-{
-    DatabasePtr system_database = DatabaseCatalog::instance().tryGetDatabase(DatabaseCatalog::SYSTEM_DATABASE);
-    if (!system_database)
-    {
-        /// TODO: add attachTableDelayed into DatabaseMemory to speedup loading
-        system_database = std::make_shared<DatabaseMemory>(DatabaseCatalog::SYSTEM_DATABASE);
-        DatabaseCatalog::instance().attachDatabase(DatabaseCatalog::SYSTEM_DATABASE, system_database);
-    }
-
-    attachSystemTablesLocal(*system_database);
-}
-
-
 void LocalServer::processQueries()
 {
     String initial_create_query = getInitialCreateTableQuery();
@@ -375,7 +375,7 @@ static void showClientVersion()
     std::cout << DBMS_NAME << " client version " << VERSION_STRING << VERSION_OFFICIAL << "." << '\n';
 }
 
-std::string LocalServer::getHelpHeader() const
+static std::string getHelpHeader()
 {
     return
         "usage: clickhouse-local [initial table definition] [--query <query>]\n"
@@ -390,7 +390,7 @@ std::string LocalServer::getHelpHeader() const
         "Either through corresponding command line parameters --table --structure --input-format and --file.";
 }
 
-std::string LocalServer::getHelpFooter() const
+static std::string getHelpFooter()
 {
     return
         "Example printing memory used by each Unix user:\n"

dbms/programs/local/LocalServer.h

@@ -37,13 +37,9 @@ class LocalServer : public Poco::Util::Application, public Loggers
     void tryInitPath();
     void applyCmdOptions();
     void applyCmdSettings();
-    void attachSystemTables();
     void processQueries();
     void setupUsers();
 
-    std::string getHelpHeader() const;
-    std::string getHelpFooter() const;
-
 protected:
     std::unique_ptr<Context> context;
 

dbms/programs/obfuscator/Obfuscator.cpp

@@ -529,13 +529,13 @@ class MarkovModel
     static constexpr CodePoint END = -2;
 
 
-    NGramHash hashContext(const CodePoint * begin, const CodePoint * end) const
+    static NGramHash hashContext(const CodePoint * begin, const CodePoint * end)
     {
         return CRC32Hash()(StringRef(reinterpret_cast<const char *>(begin), (end - begin) * sizeof(CodePoint)));
     }
 
     /// By the way, we don't have to use actual Unicode numbers. We use just arbitrary bijective mapping.
-    CodePoint readCodePoint(const char *& pos, const char * end)
+    static CodePoint readCodePoint(const char *& pos, const char * end)
     {
         size_t length = UTF8::seqLength(*pos);
 
@@ -550,7 +550,7 @@ class MarkovModel
         return res;
     }
 
-    bool writeCodePoint(CodePoint code, char *& pos, const char * end)
+    static bool writeCodePoint(CodePoint code, char *& pos, const char * end)
     {
         size_t length
             = (code & 0xFF000000) ? 4

dbms/programs/performance-test/ConfigPreprocessor.cpp

@@ -41,7 +41,7 @@ void ConfigPreprocessor::removeConfigurationsIf(
     std::vector<XMLConfigurationPtr> & configs,
     ConfigPreprocessor::FilterType filter_type,
     const Strings & values,
-    bool leave) const
+    bool leave)
 {
     auto checker = [&filter_type, &values, &leave] (XMLConfigurationPtr & config)
     {

dbms/programs/performance-test/ConfigPreprocessor.h

@@ -39,11 +39,11 @@ class ConfigPreprocessor
 
     /// Removes configurations that has a given value.
     /// If leave is true, the logic is reversed.
-    void removeConfigurationsIf(
+    static void removeConfigurationsIf(
         std::vector<XMLConfigurationPtr> & configs,
         FilterType filter_type,
         const Strings & values,
-        bool leave = false) const;
+        bool leave = false);
 
     const Strings paths;
 };

dbms/programs/performance-test/ReportBuilder.cpp

@@ -40,7 +40,7 @@ ReportBuilder::ReportBuilder(const std::string & server_version_)
 {
 }
 
-std::string ReportBuilder::getCurrentTime() const
+static std::string getCurrentTime()
 {
     return DateLUT::instance().timeToString(time(nullptr));
 }
@@ -163,7 +163,7 @@ std::string ReportBuilder::buildFullReport(
 std::string ReportBuilder::buildCompactReport(
     const PerformanceTestInfo & test_info,
     std::vector<TestStats> & stats,
-    const std::vector<std::size_t> & queries_to_run) const
+    const std::vector<std::size_t> & queries_to_run)
 {
     FormatSettings settings;
     std::ostringstream output;