fix GitHub action for cpplint style check. (k2-fsa#146)

Author: csukuangfj

.github/workflows/style_check.yml

@@ -33,7 +33,6 @@ jobs:
         run: |
           python3 -m pip install --upgrade pip
           python3 -m pip install --upgrade flake8
-          pip install cpplint==1.4.5
 
       - name: Run flake8
         shell: bash
@@ -44,6 +43,7 @@ jobs:
           # exit-zero treats all errors as warnings.
           flake8 . --count --exit-zero --max-complexity=10 --max-line-length=79 --statistics
 
+          # TODO(fangjun): build a docker for style check
           #      - name: Install cppcheck
           #        run: |
           #          cd /tmp
@@ -55,30 +55,12 @@ jobs:
           #          make -j
           #          sudo make install
 
-      - name: Create Build Directory
-        run: cmake -E make_directory ${{runner.workspace}}/build
-
-      - name: Configure CMake
-        shell: bash
-        working-directory: ${{runner.workspace}}/build
-        run: cmake -DCMAKE_CXX_CPPLINT=cpplint $GITHUB_WORKSPACE
-
       - name: Check style with cpplint
         shell: bash
         working-directory: ${{github.workspace}}
-        run: ./scripts/check_style_cpplint.sh ${{runner.workspace}}/build 1
+        run: ./scripts/check_style_cpplint.sh
 
         #      - name: Run cppcheck
         #        shell: bash
         #        working-directory: ${{github.workspace}}
         #        run: ./scripts/run_cppcheck.sh ${{runner.workspace}}/build
-
-      - name: Install clang-tidy
-        run: |
-          sudo apt-get install clang-tidy-9
-
-      - name: Run clang-tidy
-        shell: bash
-        working-directory: ${{github.workspace}}
-        run: |
-          clang-tidy-9 --quiet -p ${{runner.workspace}}/build k2/csrc/*.cc k2/python/csrc/*.cc

CMakeLists.txt

@@ -93,7 +93,6 @@ endif()
 enable_testing()
 
 list(APPEND CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/cmake)
-include(cpplint)
 include(glog)
 include(googletest)
 include(pybind11)

CPPLINT.cfg

@@ -1,2 +1,14 @@
 # Stop searching for additional config files.
 set noparent
+
+# There are lots of functions taking arguments like `Foo& foo`
+# in k2, but cpplint complains about this style.
+filter=-runtime/references
+
+# Suppress warnings for `// TODO` without a user name.
+# cpplint recommends `// TODO(username): `.
+filter=-readability/todo
+
+# cpplint does not support lambdas decorated with __host__ __device__
+# and issues a warning: You don't need a ; after a }
+filter=-readability/braces

cmake/cpplint.cmake

@@ -16,6 +16,8 @@
 
 #include <algorithm>
 #include <iostream>
+#include <utility>
+#include <vector>
 
 #include "k2/csrc/context.h"
 #include "k2/csrc/dtype.h"
@@ -400,6 +402,9 @@ class Array2 {
     Convert to possibly-different context, may require CPU/GPU transfer.
     The returned value may share the same underlying `data` memory as *this.
     This should work even for tensors with dim == 0.
+
+    Note that the returned array is contiguous in case the required context
+    is not compatible with the current context.
   */
   Array2<T> To(ContextPtr ctx) const {
     if (ctx->IsCompatible(*Context())) return *this;
@@ -459,7 +464,7 @@ class Array2 {
                                   with error.
 
   */
-  Array2(Tensor &t, bool copy_for_strides = true) {
+  explicit Array2(Tensor &t, bool copy_for_strides = true) {
     auto type = t.GetDtype();
     K2_CHECK_EQ(type, DtypeOf<T>::dtype);
     auto shape = t.GetShape();

k2/csrc/array.h

@@ -19,6 +19,7 @@
 #include <memory>
 #include <ostream>
 #include <type_traits>
+#include <vector>
 
 #include "k2/csrc/log.h"
 
@@ -215,7 +216,7 @@ class BackgroundRunner {
   void Wait();
 
  private:
-  // TODO.  My current thinking on this is for Background() to create threads
+  // TODO:  My current thinking on this is for Background() to create threads
   // that Wait() can wait on, and to have the number of threads limited by a
   // global semaphore.
 };
@@ -451,7 +452,7 @@ static thread_local CudaStreamOverride g_stream_override;
 
 class With {
  public:
-  With(cudaStream_t stream) : stream_(stream) {
+  explicit With(cudaStream_t stream) : stream_(stream) {
     g_stream_override.Push(stream_);
   }
   ~With() { g_stream_override.Pop(stream_); }
@@ -472,7 +473,7 @@ class With {
 */
 class ParallelRunner {
  public:
-  ParallelRunner(ContextPtr c) : c_(c) {}
+  explicit ParallelRunner(ContextPtr c) : c_(c) {}
 
   // create a new stream, that first syncs with stream of c_ via an event.  The
   // destructor will cause the stream of c_ to wait on this stream in the

k2/csrc/context.h

@@ -32,7 +32,7 @@ int32_t HighestBitSet(int32_t i) {
 // returns random int from [min..max]
 int32_t RandInt(int32_t min, int32_t max) {
   CHECK_GE(max, min);
-  return (min + (rand() % (max + 1 - min)));
+  return (min + (rand() % (max + 1 - min))); // NOLINT
 }
 
 // Returns random ints from a distribution that gives more weight to lower

k2/csrc/math.cu

@@ -25,13 +25,11 @@
 // Note, I'm not sure about the name of this file, they are not ops like in
 // TensorFlow, but procedures..
 
-namespace {
 // TODO(haowen): manage/load block config with some classes? then we can get
 // different configuration depending on num_elements and data type.
 // block size for matrix transpose.
-constexpr int kTransTileDim = 32;
-constexpr int kTransBlockRows = 8;
-}  // namespace
+static constexpr int kTransTileDim = 32;
+static constexpr int kTransBlockRows = 8;
 
 namespace k2 {
 // TODO(haowen): move the implementations to file `op_inl.h` or

k2/csrc/ops.h

@@ -13,16 +13,17 @@
  * See LICENSE for clarification regarding multiple authors
  */
 
+#ifndef K2_CSRC_OPS_INL_H_
+#define K2_CSRC_OPS_INL_H_
+
 #ifndef IS_IN_K2_CSRC_OPS_H_
 #error "this file is supposed to be included only by ops.h"
 #endif
 
-// No header guard for this file since it will only be included
-// in ops.h
-
 #include <cassert>
-#include <cub/cub.cuh>
+#include <cub/cub.cuh>  // NOLINT
 #include <type_traits>
+#include <vector>
 
 namespace k2 {
 
@@ -53,8 +54,8 @@ Array1<T> Append(int32_t num_arrays, const Array1<T> **src) {
     for (int32_t i = 0; i < num_arrays; i++) {
       int32_t offset = row_splits_vec[i], this_dim = src[i]->Dim();
       const int32_t *this_src_data = src[i]->Data();
-      memcpy((void *)ans_data, (const void *)this_src_data,
-             sizeof(T) * this_dim);
+      memcpy(static_cast<void *>(ans_data),
+             static_cast<const void *>(this_src_data), sizeof(T) * this_dim);
       ans_data += this_dim;
     }
   } else {
@@ -134,7 +135,7 @@ void ExclusiveSumDeref(Array1<T *> &src, Array1<T> *dest) {
   struct PtrPtr {
     T **data;
     __host__ __device__ T operator[](int32_t i) { return *(data[i]); }
-    PtrPtr(T **data) : data(data) {}
+    explicit PtrPtr(T **data) : data(data) {}
     PtrPtr(const PtrPtr &src) = default;
   };
 
@@ -212,10 +213,11 @@ Array1<T> RandUniformArray1(ContextPtr &c, int32_t dim, T min_value,
   } else if (std::is_floating_point<T>::value ||
              std::abs(min_value) > RAND_MAX || std::abs(max_value) > RAND_MAX) {
     for (int32_t i = 0; i < dim; i++)
-      data[i] = min_value + (rand() * (max_value - min_value) / RAND_MAX);
+      data[i] =
+          min_value + (rand() * (max_value - min_value) / RAND_MAX);  // NOLINT
   } else {
     for (int32_t i = 0; i < dim; i++)
-      data[i] = min_value + (rand() % (max_value + 1 - min_value));
+      data[i] = min_value + (rand() % (max_value + 1 - min_value));  // NOLINT
   }
   return temp.To(c);
 }
@@ -254,3 +256,5 @@ Array2<T> ToContiguous(const Array2<T> &src) {
 }
 
 }  // namespace k2
+
+#endif  // K2_CSRC_OPS_INL_H_

k2/csrc/ops_inl.h

@@ -12,6 +12,8 @@
 #ifndef K2_CSRC_RAGGED_H_
 #define K2_CSRC_RAGGED_H_
 
+#include <vector>
+
 #include "k2/csrc/algorithms.h"
 #include "k2/csrc/array.h"
 #include "k2/csrc/log.h"
@@ -125,7 +127,7 @@ class RaggedShape {
 
   RaggedShapeIndexIterator Iterator();
 
-  RaggedShape(std::vector<RaggedShapeDim> &axes, bool check = true)
+  explicit RaggedShape(std::vector<RaggedShapeDim> &axes, bool check = true)
       : axes_(axes) {
     if (check) Check();
   }
@@ -540,7 +542,7 @@ Ragged<T> RandomRagged(T min_value = static_cast<T>(0),
 
 }  // namespace k2
 
-// TODO(dan), include guard maybe.
+// TODO(dan): include guard maybe.
 #include "k2/csrc/ragged_inl.h"
 
 #endif  // K2_CSRC_RAGGED_H_

k2/csrc/ragged.h

@@ -13,6 +13,7 @@
 #define K2_CSRC_UTILS_H_
 
 #include <algorithm>
+#include <vector>
 
 #include "k2/csrc/context.h"
 
@@ -270,9 +271,9 @@ RowIdFromRowSplits(int32_t num_rows, const int32_t *row_splits, int32_t index,
   // auto i =
   // std::lower_bound(row_splits + 1, row_splits + num_rows + 1, index) - 1;
   // K2_DCHECK(static_cast<uint32_t>(i) < static_cast<uint32_t>(num_rows));
-  // TODO!  Implement std::lower_bound ourselves.
+  // TODO:  Implement std::lower_bound ourselves.
   // return *i;
-  return 0;  // TODO!  Does not work.
+  return 0;  // TODO:  Does not work.
 }
 
 /*
@@ -413,7 +414,7 @@ void EvalWithRedirect(cudaStream_t stream, int32_t num_jobs,
                       TaskRedirect *redirect, int32_t min_threads_per_job,
                       int32_t tot_work, int32_t target_num_loops,
                       LambdaT &lambda) {
-  // TODO..
+  // TODO:
 }
 
 __host__ __device__ __forceinline__ int32_t FloatAsInt(float f) {