Enable more checkers for clang-tidy in CI (#5738)

* CI: enable more checkers for clang-tidy

* .clang-tidy: remove cppcoreguidelines-pro-type-vararg

* CI: remove duplicate checkers

* CI: remove clang-analyzer-alpha.deadcode.*

* .clang-tidy: add performance-*

* oneflow/core/eager: remove unnecessary malloc & free

* .clang-tidy: add clang-analyzer-cplusplus.* to werror

* user_kernel: remove useless move

* quantization_aware_training: fix move return

* .clang-tidy: add google-*

* CI: fix clang tidy command

* CI: fix test

Co-authored-by: oneflow-ci-bot <69100618+oneflow-ci-bot@users.noreply.github.com>

.clang-tidy

@@ -1,4 +1,15 @@
-# maybe-* checks are only available on OneFlow custom clang-tidy and clangd
-Checks: '-*, maybe-*'
+# `maybe-*` checks are only available on OneFlow custom clang-tidy and clangd
+# `-allow-enabling-analyzer-alpha-checkers` should be passed to clang-tidy for CSA checkers named `clang-analyzer-alpha.*` (or `-allow-enabling-alpha-checkers` for run-clang-tidy.py)
+# `aggressive-binary-operation-simplification` should be enabled (via `-Xclang -analyzer-config -Xclang aggressive-binary-operation-simplification=true` in clang)
+# there is some problem in `clang-analyzer-alpha.clone.*`, so do not enable it
+# `clang-analyzer-alpha.deadcode.*` is just too verbose to enable
+Checks: '-*, maybe-*, clang-analyzer-core.*, clang-analyzer-cplusplus.*, clang-analyzer-nullability.*, clang-analyzer-deadcode.*, clang-analyzer-security.*, clang-analyzer-optin.cplusplus.*, clang-analyzer-optin.performance.*, clang-analyzer-alpha.core.*, clang-analyzer-alpha.cplusplus.*, clang-analyzer-alpha.security.*, cppcoreguidelines-avoid-goto, cppcoreguidelines-init-variables, cppcoreguidelines-interfaces-global-init, cppcoreguidelines-no-malloc, cppcoreguidelines-prefer-member-initializer, cppcoreguidelines-pro-type-member-init, cppcoreguidelines-pro-type-static-cast-downcast, cppcoreguidelines-slicing, cppcoreguidelines-special-member-functions, performance-unnecessary-value-param, performance-unnecessary-copy-initialization, performance-noexcept-move-constructor, performance-no-automatic-move, performance-move-const-arg, performance-implicit-conversion-in-loop, performance-for-range-copy, google-default-arguments, google-global-names-in-headers, google-explicit-constructor'
 # TODO: treat all maybe warnings as errors when existing warnings are all fixed
-WarningsAsErrors: 'maybe-unused'
+WarningsAsErrors: 'maybe-unused, clang-analyzer-nullability.*, clang-analyzer-cplusplus.*, performance-implicit-conversion-in-loop, performance-move-const-arg, performance-no-automatic-move, performance-noexcept-move-constructor, google-default-arguments, google-global-names-in-headers'
+
+CheckOptions:
+  # `cppcoreguidelines-special-member-functions` is enabled, refer to https://en.cppreference.com/w/cpp/language/rule_of_three
+  - key:             cppcoreguidelines-special-member-functions.AllowSoleDefaultDtor
+    value:           True
+  - key:             performance-move-const-arg.CheckTriviallyCopyableMove
+    value:           False

.github/workflows/simple.yml

@@ -62,7 +62,7 @@ jobs:
             -DBUILD_TESTING=ON \
             -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
           cd ..
-          ./run-clang-tidy.py -clang-tidy-binary ./clang-tidy-489012f-x86_64.AppImage -p build -quiet
+          ./run-clang-tidy.py -clang-tidy-binary ./clang-tidy-489012f-x86_64.AppImage -p build -quiet -allow-enabling-alpha-checkers -extra-arg="-Xclang" -extra-arg="-analyzer-config" -extra-arg="-Xclang" -extra-arg="aggressive-binary-operation-simplification=true" '^((?!third_party_install).)+(?<!cfg.cpp)(?<!pb.cc)$'
 
   hosted:
     name: CPU-only

.github/workflows/test.yml

@@ -761,4 +761,4 @@ jobs:
             -DBUILD_TESTING=ON \
             -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
           cd ..
-          git diff -U0 ${{ github.event.pull_request.base.sha }} | ./clang-tidy-diff.py -clang-tidy-binary ./clang-tidy-489012f-x86_64.AppImage -path build -quiet -j $(nproc) -p1
+          git diff -U0 ${{ github.event.pull_request.base.sha }} | ./clang-tidy-diff.py -clang-tidy-binary ./clang-tidy-489012f-x86_64.AppImage -path build -allow-enabling-alpha-checkers -j $(nproc) -p1 -extra-arg="-Xclang" -extra-arg="-analyzer-config" -extra-arg="-Xclang" -extra-arg="aggressive-binary-operation-simplification=true"

oneflow/core/eager/eager_blob_object.cpp

@@ -47,9 +47,7 @@ Maybe<void> EagerBlobObject::InitBlob() {
   {
     header_buffer_.reset();
     int64_t header_byte_size = blob_desc_.AlignedByteSizeOfBlobHeader();
-    const auto& FreeHeader = [header_byte_size](char* dptr) { std::free(dptr); };
-    char* ptr = reinterpret_cast<char*>(std::malloc(header_byte_size));
-    header_buffer_ = std::unique_ptr<char, std::function<void(char*)>>(ptr, FreeHeader);
+    header_buffer_ = std::make_unique<char[]>(header_byte_size);
   }
   blob_.reset(new Blob(*mem_case_, &blob_desc_, header_buffer_.get(), nullptr));
   return Maybe<void>::Ok();

oneflow/core/eager/eager_blob_object.h

@@ -90,7 +90,7 @@ class EagerBlobObject final : public BlobObject {
                   const Optional<LocalDepObject*>& dep_object);
 
   std::unique_ptr<Blob> blob_;
-  std::unique_ptr<char, std::function<void(char*)>> header_buffer_;
+  std::unique_ptr<char[]> header_buffer_;
   std::shared_ptr<TensorBuffer> tensor_buffer_;
   std::size_t blob_body_bytes_;
   std::unique_ptr<MemoryAllocator> non_pod_initer_;

oneflow/core/job_rewriter/quantization_aware_training.cpp

@@ -216,13 +216,13 @@ Maybe<bool> InsertQuantOpAfterInt8Ops4QatConfig(const QatConfig& qat_config) {
 user_op::UserOpConfWrapper MultiplyOp(const std::string& name, const std::string& x,
                                       const std::string& y, const int64_t scope_symbol_id,
                                       OpConfMap* inserted_ops) {
-  const auto op_wrapper = user_op::UserOpConfWrapperBuilder(name)
-                              .Op("broadcast_mul")
-                              .Input("x", x)
-                              .Input("y", y)
-                              .Output("z")
-                              .ScopeSymbolId(scope_symbol_id)
-                              .Build();
+  auto op_wrapper = user_op::UserOpConfWrapperBuilder(name)
+                        .Op("broadcast_mul")
+                        .Input("x", x)
+                        .Input("y", y)
+                        .Output("z")
+                        .ScopeSymbolId(scope_symbol_id)
+                        .Build();
   (*inserted_ops)[name] = op_wrapper.op_conf();
   return op_wrapper;
 }

oneflow/core/kernel/user_kernel.cpp

@@ -524,7 +524,7 @@ class UserKernelComputeContext final : public user_op::KernelComputeContext {
                                     const JobDesc& job_desc)
       : user_op_conf_(kernel_conf.op_attribute().op_conf()),
         device_ctx_(device_ctx),
-        base_ctx_(std::move(UserKernelBaseContext(kernel_conf, job_desc))) {
+        base_ctx_(kernel_conf, job_desc) {
     auto InitInOrOut = [&](const PbMap<std::string, UserOpConf::ListString>& arg_map) {
       for (const auto& it : arg_map) {
         const std::string& arg_name = it.first;