Add more tidy checks and apply fixes (#4466)

cyyever · facebook-github-bot · commit b16ce1846350 · 2025-07-09T21:53:36.000-07:00
Summary: Pull Request resolved: #4466 X-link: facebookresearch/FBGEMM#1525 Almost all fixes are replacing ``std::endl`` with '\n' and replacing `std::bind` with lambdas. Pull Request resolved: #4461 Reviewed By: cthi Differential Revision: D78024382 Pulled By: q10 fbshipit-source-id: 27301c44878863fd628d09ff5843ddf9093ab68f
diff --git a/.clang-tidy b/.clang-tidy
@@ -7,6 +7,8 @@ InheritParentConfig: true
 # @nolint
 Checks: '
 -*,
+clang-analyzer*,
+clang-diagnostic-missing-prototypes,
 cppcoreguidelines-init-variables,
 bugprone-argument-comment,
 misc-use-internal-linkage,
@@ -21,7 +23,6 @@ modernize*,
 -modernize-use-integer-sign-comparison
 -modernize-use-nodiscard,
 performance*,
--performance-avoid-endl
 '
 CheckOptions:
  - key: facebook-cuda-safe-api-call-check.HandlerName
diff --git a/src/EmbeddingSpMDM.cc b/src/EmbeddingSpMDM.cc
@@ -997,7 +997,7 @@ GenEmbeddingSpMDMLookup<
         }
 
         if (err) {
-          std::cout << "Error: in fn add" << std::endl;
+          std::cout << "Error: in fn add" << '\n';
           return nullptr;
         }
 
diff --git a/src/EmbeddingSpMDMAutovec.cc b/src/EmbeddingSpMDMAutovec.cc
@@ -15,7 +15,7 @@
 #include "fbgemm/FbgemmBuild.h"
 #include "fbgemm/FloatConversion.h"
 
-#include <math.h>
+#include <cmath>
 
 #include <algorithm>
 #include <array>
diff --git a/src/EmbeddingSpMDMNBit.cc b/src/EmbeddingSpMDMNBit.cc
@@ -999,7 +999,7 @@ GenEmbeddingSpMDMNBitLookup<
           err = runtime().add(&fn, &code);
         }
         if (err) {
-          cout << "Error: in fn add" << endl;
+          cout << "Error: in fn add" << '\n';
           return nullptr;
         }
 
diff --git a/src/EmbeddingStatsTracker.cc b/src/EmbeddingStatsTracker.cc
@@ -53,8 +53,7 @@ void EmbeddingStatsTracker::recordPattern(
     }
     for (const auto& pair : tables_) {
       const auto& pattern = pair.first;
-      logFile_ << pattern.toString() << "freq=" << pair.second << ";"
-               << std::endl;
+      logFile_ << pattern.toString() << "freq=" << pair.second << ";" << '\n';
     }
     logFile_.flush();
     logFile_.close();
diff --git a/src/Fbgemm.cc b/src/Fbgemm.cc
@@ -227,29 +227,39 @@ bool fbgemmOptimizedGConv(const conv_param_t<SPATIAL_DIM>& conv_p) {
       std::all_of(
              conv_p.K.begin(),
              conv_p.K.end(),
-             std::bind(areEqual, std::placeholders::_1, 3)) &&
+             [areEqual](auto&& PH1) {
+               return areEqual(std::forward<decltype(PH1)>(PH1), 3);
+             }) &&
 
       std::all_of(
              conv_p.pad.begin(),
              conv_p.pad.end(),
-             std::bind(areEqual, std::placeholders::_1, 1)) &&
+             [areEqual](auto&& PH1) {
+               return areEqual(std::forward<decltype(PH1)>(PH1), 1);
+             }) &&
 
       std::all_of(
              conv_p.dilation.begin(),
              conv_p.dilation.end(),
-             std::bind(areEqual, std::placeholders::_1, 1)) &&
+             [areEqual](auto&& PH1) {
+               return areEqual(std::forward<decltype(PH1)>(PH1), 1);
+             }) &&
 
       // Height/Width strides should be the same and
       // should be either 1 or 2
       // Temporal stride can be anything.
       (std::all_of(
            conv_p.stride.begin() + SPATIAL_DIM - 2,
            conv_p.stride.end(),
-           std::bind(areEqual, std::placeholders::_1, 1)) ||
+           [areEqual](auto&& PH1) {
+             return areEqual(std::forward<decltype(PH1)>(PH1), 1);
+           }) ||
        std::all_of(
            conv_p.stride.begin() + SPATIAL_DIM - 2,
            conv_p.stride.end(),
-           std::bind(areEqual, std::placeholders::_1, 2))) &&
+           [areEqual](auto&& PH1) {
+             return areEqual(std::forward<decltype(PH1)>(PH1), 2);
+           })) &&
       !conv_p.transposed;
 }
 
diff --git a/src/FbgemmI64.cc b/src/FbgemmI64.cc
@@ -382,7 +382,7 @@ CodeGenBase<int64_t, int64_t, int64_t, int64_t>::getOrCreate(
       err = runtime().add(&fn, &code);
     }
     if (err) {
-      cout << "Error: in fn add" << endl;
+      cout << "Error: in fn add" << '\n';
       return nullptr;
     }
 
diff --git a/src/GenerateI8Depthwise.cc b/src/GenerateI8Depthwise.cc
@@ -564,7 +564,7 @@ GenI8Depthwise::jit_kernel_signature GenI8Depthwise::getOrCreate(
       err = runtime().add(&fn, &code);
     }
     if (err) {
-      std::cout << "Error: in fn add" << std::endl;
+      std::cout << "Error: in fn add" << '\n';
       return nullptr;
     }
 
diff --git a/src/GenerateKernelDirectConvU8S8S32ACC32.cc b/src/GenerateKernelDirectConvU8S8S32ACC32.cc
@@ -408,7 +408,7 @@ DirectConvCodeGenBase<uint8_t, int8_t, int32_t, int32_t>::getOrCreateDirectConv(
       err = runtime().add(&fn, &code);
     }
     if (err) {
-      std::cout << "Error: in fn add" << std::endl;
+      std::cout << "Error: in fn add" << '\n';
       return nullptr;
     }
 
@@ -777,7 +777,7 @@ DirectConvCodeGenBase<uint8_t, int8_t, int32_t, int32_t>::
       err = runtime().add(&fn, &code);
     }
     if (err) {
-      std::cout << "Error: in fn add" << std::endl;
+      std::cout << "Error: in fn add" << '\n';
       return nullptr;
     }
 
diff --git a/src/GenerateKernelU8S8S32ACC16.cc b/src/GenerateKernelU8S8S32ACC16.cc
@@ -329,7 +329,7 @@ CodeGenBase<uint8_t, int8_t, int32_t, int16_t>::getOrCreate<inst_set_t::avx2>(
       err = runtime().add(&fn, &code);
     }
     if (err) {
-      std::cout << "Error: in fn add" << std::endl;
+      std::cout << "Error: in fn add" << '\n';
       return nullptr;
     }
 
diff --git a/src/GenerateKernelU8S8S32ACC16Avx512.cc b/src/GenerateKernelU8S8S32ACC16Avx512.cc
@@ -352,7 +352,7 @@ CodeGenBase<uint8_t, int8_t, int32_t, int16_t>::getOrCreate(
       err = runtime().add(&fn, &code);
     }
     if (err) {
-      std::cout << "Error: in fn add" << std::endl;
+      std::cout << "Error: in fn add" << '\n';
       return nullptr;
     }
 
diff --git a/src/GenerateKernelU8S8S32ACC32.cc b/src/GenerateKernelU8S8S32ACC32.cc
@@ -363,7 +363,7 @@ CodeGenBase<uint8_t, int8_t, int32_t, int32_t>::getOrCreate(
       err = runtime().add(&fn, &code);
     }
     if (err) {
-      std::cout << "Error: in fn add" << std::endl;
+      std::cout << "Error: in fn add" << '\n';
       return nullptr;
     }
 
diff --git a/src/GenerateKernelU8S8S32ACC32Avx512VNNI.cc b/src/GenerateKernelU8S8S32ACC32Avx512VNNI.cc
@@ -358,7 +358,7 @@ CodeGenBase<uint8_t, int8_t, int32_t, int32_t>::getOrCreate(
       err = runtime().add(&fn, &code);
     }
     if (err) {
-      std::cout << "Error: in fn add" << std::endl;
+      std::cout << "Error: in fn add" << '\n';
       return nullptr;
     }
 
diff --git a/src/GroupwiseConv.cc b/src/GroupwiseConv.cc
@@ -307,7 +307,7 @@ jit_conv_kernel_fp GenConvKernel<SPATIAL_DIM, INST_SET>::getOrCreate() {
   }
 
   if (err) {
-    cout << "Error: in fn add" << endl;
+    cout << "Error: in fn add" << '\n';
     return nullptr;
   }
 
diff --git a/src/PackAMatrix.cc b/src/PackAMatrix.cc
@@ -204,9 +204,9 @@ void PackAMatrix<T, accT>::printPackedMatrix(const std::string& name) {
         std::cout << std::setw(5) << val << " ";
       }
     }
-    std::cout << std::endl;
+    std::cout << '\n';
   }
-  std::cout << std::endl;
+  std::cout << '\n';
 }
 
 template class PackAMatrix<uint8_t, int32_t>;
diff --git a/src/PackAWithIm2Col.cc b/src/PackAWithIm2Col.cc
@@ -718,9 +718,9 @@ void PackAWithIm2Col<T, accT, SPATIAL_DIM>::printPackedMatrix(
         std::cout << std::setw(5) << val << " ";
       }
     }
-    std::cout << std::endl;
+    std::cout << '\n';
   }
-  std::cout << std::endl;
+  std::cout << '\n';
 }
 
 template <typename T, typename accT, int SPATIAL_DIM>
diff --git a/src/PackAWithQuantRowOffset.cc b/src/PackAWithQuantRowOffset.cc
@@ -225,9 +225,9 @@ void PackAWithQuantRowOffset<T, accT>::printPackedMatrix(
         std::cout << std::setw(5) << val << " ";
       }
     }
-    std::cout << std::endl;
+    std::cout << '\n';
   }
-  std::cout << std::endl;
+  std::cout << '\n';
 }
 
 template <typename T, typename accT>
diff --git a/src/PackAWithRowOffset.cc b/src/PackAWithRowOffset.cc
@@ -203,9 +203,9 @@ void PackAWithRowOffset<T, accT>::printPackedMatrix(const std::string& name) {
         std::cout << std::setw(5) << val << " ";
       }
     }
-    std::cout << std::endl;
+    std::cout << '\n';
   }
-  std::cout << std::endl;
+  std::cout << '\n';
 }
 
 template <typename T, typename accT>
diff --git a/src/PackBMatrix.cc b/src/PackBMatrix.cc
@@ -400,12 +400,12 @@ void PackBMatrix<T, accT>::printPackedMatrix(
         g *
             BaseType::packedBufferSize(
                 BaseType::numPackedRows(), BaseType::numPackedCols(), params);
-    std::cout << "group: " << g << std::endl;
+    std::cout << "group: " << g << '\n';
     for (auto nr = 0; nr < BaseType::blockRows(); ++nr) {
       auto rows = (nr == BaseType::blockRows() - 1) ? BaseType::lastBrow()
                                                     : BaseType::blockRowSize();
       for (auto nc = 0; nc < BaseType::blockCols(); ++nc) {
-        std::cout << "block:" << nr << ", " << nc << std::endl;
+        std::cout << "block:" << nr << ", " << nc << '\n';
         auto cols = (nc == BaseType::blockCols() - 1)
             ? BaseType::lastBcol()
             : BaseType::blockColSize();
@@ -424,9 +424,9 @@ void PackBMatrix<T, accT>::printPackedMatrix(
               std::cout << std::setw(5) << val << " ";
             }
           }
-          std::cout << std::endl;
+          std::cout << '\n';
         }
-        std::cout << std::endl;
+        std::cout << '\n';
       }
     }
   }
diff --git a/src/PackWeightsForConv.cc b/src/PackWeightsForConv.cc
@@ -139,7 +139,7 @@ std::string PackWeightsForConv<SPATIAL_DIM, T, accT>::mismatchingParams(
     return out;
   };
 
-  auto combineInt = [&combineStr](std::string id, int int1, int int2) {
+  auto combineInt = [&combineStr](const std::string& id, int int1, int int2) {
     return combineStr(
         std::move(id), std::to_string(int1), std::to_string(int2));
   };
diff --git a/src/RefImplementations.cc b/src/RefImplementations.cc
@@ -13,7 +13,7 @@
 #include "fbgemm/FbgemmConvert.h"
 #include "fbgemm/FloatConversion.h"
 
-#include <math.h>
+#include <cmath>
 
 #include <algorithm>
 #include <cassert>
@@ -1949,7 +1949,7 @@ int rowwise_sparse_adagrad_fused_ref(
   int vlen = emu_vector_size;
   if (vlen != 8 && vlen != 16) {
     // Raise error as it may cause buffer overflow
-    cerr << "Not supported emu_vector_size: " << emu_vector_size << endl;
+    cerr << "Not supported emu_vector_size: " << emu_vector_size << '\n';
     return 0;
   }
 
diff --git a/src/RowWiseSparseAdagradFused.cc b/src/RowWiseSparseAdagradFused.cc
@@ -760,7 +760,7 @@ typename ReturnFunctionSignature<indxType, offsetType, dataType>::
           err = runtime().add(&fn, &code);
         }
         if (err) {
-          cout << "Error: in fn add" << endl;
+          cout << "Error: in fn add" << '\n';
           return nullptr;
         }
 
diff --git a/src/SparseAdagrad.cc b/src/SparseAdagrad.cc
@@ -776,7 +776,7 @@ GenSparseAdagrad<indxType, instSet>::getOrCreate(
           err = runtime().add(&fn, &code);
         }
         if (err) {
-          std::cout << "Error: in fn add" << std::endl;
+          std::cout << "Error: in fn add" << '\n';
           return nullptr;
         }
 
diff --git a/src/Utils.cc b/src/Utils.cc
@@ -65,10 +65,10 @@ int compare_buffers(
     for (int j = 0; j < n; ++j) {
       T reference = ref[i * ld + j], actual = test[i * ld + j];
       if (std::abs(reference - actual) > atol) {
-        std::cout << "\tmismatch at (" << i << ", " << j << ")" << std::endl;
+        std::cout << "\tmismatch at (" << i << ", " << j << ")" << '\n';
         if constexpr (std::is_integral_v<T>) {
           std::cout << "\t  reference:" << static_cast<int64_t>(reference)
-                    << " test:" << static_cast<int64_t>(actual) << std::endl;
+                    << " test:" << static_cast<int64_t>(actual) << '\n';
         } else {
           std::cout << "\t  reference:" << reference << " test:" << actual
                     << std::endl;
@@ -103,7 +103,7 @@ void printMatrix(
   // R: number of rows in op(inp)
   // C: number of cols in op(inp)
   // ld: leading dimension in inp
-  std::cout << name << ":" << "[" << R << ", " << C << "]" << std::endl;
+  std::cout << name << ":" << "[" << R << ", " << C << "]" << '\n';
   bool tr = (op == matrix_op_t::Transpose);
   for (size_t r = 0; r < R; ++r) {
     for (size_t c = 0; c < C; ++c) {
@@ -114,7 +114,7 @@ void printMatrix(
         std::cout << std::setw(5) << res << " ";
       }
     }
-    std::cout << std::endl;
+    std::cout << '\n';
   }
 }
 
diff --git a/src/spmmUtils.cc b/src/spmmUtils.cc
@@ -170,11 +170,11 @@ vector<vector<int>> getSparseMatrixShapes() {
       {51, 1024, 4096},
     };
     shapes.insert(shapes.end(), roberta_shapes.begin(), roberta_shapes.end() );
-    cout << "RoBERTa shapes included." << endl;
+    cout << "RoBERTa shapes included." << '\n';
   }
   else {
     cout << "RoBERTa shapes not included. " <<
-      "To include, add \"INCLUDE_ROBERTA=1\" as an env variable." << endl;
+      "To include, add \"INCLUDE_ROBERTA=1\" as an env variable." << '\n';
   }
 
   // LSTM shapes
@@ -189,11 +189,11 @@ vector<vector<int>> getSparseMatrixShapes() {
       {18, 2560, 720},
     };
     shapes.insert(shapes.end(), lstm_shapes.begin(), lstm_shapes.end() );
-    cout << "LSTM shapes included." << endl;
+    cout << "LSTM shapes included." << '\n';
   }
   else {
     cout << "LSTM shapes not included. " <<
-      "To include, add \"INCLUDE_LSTM=1\" as an env variable." << endl;
+      "To include, add \"INCLUDE_LSTM=1\" as an env variable." << '\n';
   }
 
   // RNNT shapes
@@ -241,11 +241,11 @@ vector<vector<int>> getSparseMatrixShapes() {
       {64, 256, 4500},
     };
     shapes.insert(shapes.end(), rnnt_shapes.begin(), rnnt_shapes.end() );
-    cout << "rnnt shapes included." << endl;
+    cout << "rnnt shapes included." << '\n';
   }
   else {
     cout << "RNNT shapes not included. " <<
-      "To include, add \"INCLUDE_RNNT=1\" as an env variable." << endl;
+      "To include, add \"INCLUDE_RNNT=1\" as an env variable." << '\n';
   }
   // clang-format on
   return shapes;

Original file line number	Diff line number	Diff line change
`@@ -997,7 +997,7 @@ GenEmbeddingSpMDMLookup<`
`997`	`997`	`}`
`998`	`998`
`999`	`999`	`if (err) {`
`1000`		`- std::cout << "Error: in fn add" << std::endl;`
	`1000`	`+ std::cout << "Error: in fn add" << '\n';`
`1001`	`1001`	`return nullptr;`
`1002`	`1002`	`}`
`1003`	`1003`
Original file line number	Diff line number	Diff line change
`@@ -999,7 +999,7 @@ GenEmbeddingSpMDMNBitLookup<`
`999`	`999`	`err = runtime().add(&fn, &code);`
`1000`	`1000`	`}`
`1001`	`1001`	`if (err) {`
`1002`		`- cout << "Error: in fn add" << endl;`
	`1002`	`+ cout << "Error: in fn add" << '\n';`
`1003`	`1003`	`return nullptr;`
`1004`	`1004`	`}`
`1005`	`1005`
Original file line number	Diff line number	Diff line change
`@@ -53,8 +53,7 @@ void EmbeddingStatsTracker::recordPattern(`
`53`	`53`	`}`
`54`	`54`	`for (const auto& pair : tables_) {`
`55`	`55`	`const auto& pattern = pair.first;`
`56`		`- logFile_ << pattern.toString() << "freq=" << pair.second << ";"`
`57`		`- << std::endl;`
	`56`	`+ logFile_ << pattern.toString() << "freq=" << pair.second << ";" << '\n';`
`58`	`57`	`}`
`59`	`58`	`logFile_.flush();`
`60`	`59`	`logFile_.close();`
Original file line number	Diff line number	Diff line change
`@@ -382,7 +382,7 @@ CodeGenBase<int64_t, int64_t, int64_t, int64_t>::getOrCreate(`
`382`	`382`	`err = runtime().add(&fn, &code);`
`383`	`383`	`}`
`384`	`384`	`if (err) {`
`385`		`- cout << "Error: in fn add" << endl;`
	`385`	`+ cout << "Error: in fn add" << '\n';`
`386`	`386`	`return nullptr;`
`387`	`387`	`}`
`388`	`388`
Original file line number	Diff line number	Diff line change
`@@ -564,7 +564,7 @@ GenI8Depthwise::jit_kernel_signature GenI8Depthwise::getOrCreate(`
`564`	`564`	`err = runtime().add(&fn, &code);`
`565`	`565`	`}`
`566`	`566`	`if (err) {`
`567`		`- std::cout << "Error: in fn add" << std::endl;`
	`567`	`+ std::cout << "Error: in fn add" << '\n';`
`568`	`568`	`return nullptr;`
`569`	`569`	`}`
`570`	`570`
Original file line number	Diff line number	Diff line change
`@@ -408,7 +408,7 @@ DirectConvCodeGenBase<uint8_t, int8_t, int32_t, int32_t>::getOrCreateDirectConv(`
`408`	`408`	`err = runtime().add(&fn, &code);`
`409`	`409`	`}`
`410`	`410`	`if (err) {`
`411`		`- std::cout << "Error: in fn add" << std::endl;`
	`411`	`+ std::cout << "Error: in fn add" << '\n';`
`412`	`412`	`return nullptr;`
`413`	`413`	`}`
`414`	`414`
`@@ -777,7 +777,7 @@ DirectConvCodeGenBase<uint8_t, int8_t, int32_t, int32_t>::`
`777`	`777`	`err = runtime().add(&fn, &code);`
`778`	`778`	`}`
`779`	`779`	`if (err) {`
`780`		`- std::cout << "Error: in fn add" << std::endl;`
	`780`	`+ std::cout << "Error: in fn add" << '\n';`
`781`	`781`	`return nullptr;`
`782`	`782`	`}`
`783`	`783`
Original file line number	Diff line number	Diff line change
`@@ -329,7 +329,7 @@ CodeGenBase<uint8_t, int8_t, int32_t, int16_t>::getOrCreate<inst_set_t::avx2>(`
`329`	`329`	`err = runtime().add(&fn, &code);`
`330`	`330`	`}`
`331`	`331`	`if (err) {`
`332`		`- std::cout << "Error: in fn add" << std::endl;`
	`332`	`+ std::cout << "Error: in fn add" << '\n';`
`333`	`333`	`return nullptr;`
`334`	`334`	`}`
`335`	`335`