Merge pull request #1945 from teju85/fea-ext-clang-tidy

[REVIEW] Enable clang tidy on cuML c++ sources

CHANGELOG.md

@@ -5,6 +5,7 @@
 ## Improvements
 
 - PR #2310: Pinning ucx-py to 0.14 to make 0.15 CI pass
+- PR #1945: enable clang tidy
 
 ## Bug Fixes
 

ci/checks/style.sh

@@ -1,15 +1,19 @@
 #!/bin/bash
-# Copyright (c) 2019, NVIDIA CORPORATION.
+# Copyright (c) 2019-2020, NVIDIA CORPORATION.
 #####################
 # cuML Style Tester #
 #####################
 
 # Ignore errors and set path
 set +e
-PATH=/conda/bin:$PATH
+export PATH=/conda/bin:/usr/local/cuda/bin:$PATH
 
-# Activate common conda env
+# Activate common conda env and install any dependencies needed
 source activate gdf
+cd $WORKSPACE
+export GIT_DESCRIBE_TAG=`git describe --tags`
+export MINOR_VERSION=`echo $GIT_DESCRIBE_TAG | grep -o -E '([0-9]+\.[0-9]+)'`
+conda install "ucx-py=${MINOR_VERSION}"
 
 # Run flake8 and get results/return code
 FLAKE=`flake8 --exclude=cpp,thirdparty,__init__.py,versioneer.py && flake8 --config=python/.flake8.cython`
@@ -66,7 +70,6 @@ else
 fi
 
 # Check for a consistent code format
-# TODO: keep adding more dirs when we add more source folders in cuml
 FORMAT=`python cpp/scripts/run-clang-format.py 2>&1`
 FORMAT_RETVAL=$?
 if [ "$RETVAL" = "0" ]; then
@@ -82,4 +85,40 @@ else
   echo -e "\n\n>>>> PASSED: clang format check\n\n"
 fi
 
+# clang-tidy check
+# NOTE:
+#   explicitly pass GPU_ARCHS flag to avoid having to evaluate gpu archs
+# because there's no GPU on the CI machine where this script runs!
+# NOTE:
+#   also, sync all dependencies as they'll be needed by clang-tidy to find
+# relevant headers
+function setup_and_run_clang_tidy() {
+    local LD_LIBRARY_PATH_CACHED=$LD_LIBRARY_PATH
+    export LD_LIBRARY_PATH=$CONDA_PREFIX/lib:$LD_LIBRARY_PATH
+    mkdir cpp/build && \
+        cd cpp/build && \
+        cmake -DGPU_ARCHS=70 \
+              -DBLAS_LIBRARIES=${CONDA_PREFIX}/lib/libopenblas.so.0 \
+              .. && \
+        make treelite && \
+        cd ../.. && \
+        python cpp/scripts/run-clang-tidy.py
+    export LD_LIBRARY_PATH=$LD_LIBRARY_PATH_CACHED
+}
+TIDY=`setup_and_run_clang_tidy 2>&1`
+TIDY_RETVAL=$?
+if [ "$RETVAL" = "0" ]; then
+  RETVAL=$TIDY_RETVAL
+fi
+
+# Output results if failure otherwise show pass
+if [ "$TIDY_RETVAL" != "0" ]; then
+  echo -e "\n\n>>>> FAILED: clang tidy check; begin output\n\n"
+  echo -e "$TIDY"
+  echo -e "\n\n>>>> FAILED: clang tidy check; end output\n\n"
+else
+  echo -e "\n\n>>>> PASSED: clang tidy check\n\n"
+fi
+
+
 exit $RETVAL

ci/gpu/build.sh

@@ -115,7 +115,6 @@ python setup.py install
 
 cd $WORKSPACE
 
-
 ################################################################################
 # TEST - Run GoogleTest and py.tests for libcuml and cuML
 ################################################################################

cpp/.clang-tidy

@@ -0,0 +1,145 @@
+---
+Checks:          'clang-diagnostic-*,clang-analyzer-*,-modernize-*,-clang-diagnostic-#pragma-messages,-readability-identifier-naming,-clang-diagnostic-switch'
+WarningsAsErrors: '*'
+HeaderFilterRegex: ''
+AnalyzeTemporaryDtors: false
+FormatStyle:     none
+CheckOptions:    
+  - key:             cert-dcl16-c.NewSuffixes
+    value:           'L;LL;LU;LLU'
+  - key:             cppcoreguidelines-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic
+    value:           '1'
+  - key:             modernize-loop-convert.MaxCopySize
+    value:           '16'
+  - key:             modernize-loop-convert.MinConfidence
+    value:           reasonable
+  - key:             modernize-loop-convert.NamingStyle
+    value:           CamelCase
+  - key:             modernize-pass-by-value.IncludeStyle
+    value:           llvm
+  - key:             modernize-pass-by-value.ValuesOnly
+    value:           '0'
+  - key:             modernize-replace-auto-ptr.IncludeStyle
+    value:           llvm
+  - key:             modernize-replace-random-shuffle.IncludeStyle
+    value:           llvm
+  - key:             modernize-use-auto.MinTypeNameLength
+    value:           '5'
+  - key:             modernize-use-auto.RemoveStars
+    value:           '0'
+  - key:             modernize-use-default-member-init.IgnoreMacros
+    value:           '1'
+  - key:             modernize-use-default-member-init.UseAssignment
+    value:           '0'
+  - key:             modernize-use-emplace.ContainersWithPushBack
+    value:           '::std::vector;::std::list;::std::deque'
+  - key:             modernize-use-emplace.SmartPointers
+    value:           '::std::shared_ptr;::std::unique_ptr;::std::auto_ptr;::std::weak_ptr'
+  - key:             modernize-use-emplace.TupleMakeFunctions
+    value:           '::std::make_pair;::std::make_tuple'
+  - key:             modernize-use-emplace.TupleTypes
+    value:           '::std::pair;::std::tuple'
+  - key:             modernize-use-equals-default.IgnoreMacros
+    value:           '1'
+  - key:             modernize-use-equals-delete.IgnoreMacros
+    value:           '1'
+  - key:             modernize-use-nodiscard.ReplacementString
+    value:           '[[nodiscard]]'
+  - key:             modernize-use-noexcept.ReplacementString
+    value:           ''
+  - key:             modernize-use-noexcept.UseNoexceptFalse
+    value:           '1'
+  - key:             modernize-use-nullptr.NullMacros
+    value:           'NULL'
+  - key:             modernize-use-transparent-functors.SafeMode
+    value:           '0'
+  - key:             modernize-use-using.IgnoreMacros
+    value:           '1'
+  - key:             readability-identifier-naming.ClassCase
+    value:           CamelCase
+  - key:             readability-identifier-naming.ClassPrefix
+    value:           ''
+  - key:             readability-identifier-naming.ClassSuffix
+    value:           ''
+  - key:             readability-identifier-naming.ConstexprVariableCase
+    value:           CamelCase
+  - key:             readability-identifier-naming.ConstexprVariablePrefix
+    value:           k
+  - key:             readability-identifier-naming.ConstexprVariableSuffix
+    value:           ''
+  - key:             readability-identifier-naming.EnumCase
+    value:           CamelCase
+  - key:             readability-identifier-naming.EnumConstantPrefix
+    value:           k
+  - key:             readability-identifier-naming.EnumConstantSuffix
+    value:           ''
+  - key:             readability-identifier-naming.EnumPrefix
+    value:           ''
+  - key:             readability-identifier-naming.EnumSuffix
+    value:           ''
+  - key:             readability-identifier-naming.FunctionCase
+    value:           CamelCase
+  - key:             readability-identifier-naming.FunctionPrefix
+    value:           ''
+  - key:             readability-identifier-naming.FunctionSuffix
+    value:           ''
+  - key:             readability-identifier-naming.GlobalConstantCase
+    value:           CamelCase
+  - key:             readability-identifier-naming.GlobalConstantPrefix
+    value:           k
+  - key:             readability-identifier-naming.GlobalConstantSuffix
+    value:           ''
+  - key:             readability-identifier-naming.IgnoreFailedSplit
+    value:           '0'
+  - key:             readability-identifier-naming.MemberCase
+    value:           lower_case
+  - key:             readability-identifier-naming.MemberPrefix
+    value:           ''
+  - key:             readability-identifier-naming.MemberSuffix
+    value:           ''
+  - key:             readability-identifier-naming.NamespaceCase
+    value:           lower_case
+  - key:             readability-identifier-naming.NamespacePrefix
+    value:           ''
+  - key:             readability-identifier-naming.NamespaceSuffix
+    value:           ''
+  - key:             readability-identifier-naming.PrivateMemberPrefix
+    value:           ''
+  - key:             readability-identifier-naming.PrivateMemberSuffix
+    value:           _
+  - key:             readability-identifier-naming.ProtectedMemberPrefix
+    value:           ''
+  - key:             readability-identifier-naming.ProtectedMemberSuffix
+    value:           _
+  - key:             readability-identifier-naming.StaticConstantCase
+    value:           CamelCase
+  - key:             readability-identifier-naming.StaticConstantPrefix
+    value:           k
+  - key:             readability-identifier-naming.StaticConstantSuffix
+    value:           ''
+  - key:             readability-identifier-naming.StructCase
+    value:           CamelCase
+  - key:             readability-identifier-naming.StructPrefix
+    value:           ''
+  - key:             readability-identifier-naming.StructSuffix
+    value:           ''
+  - key:             readability-identifier-naming.TypeAliasCase
+    value:           CamelCase
+  - key:             readability-identifier-naming.TypeAliasPrefix
+    value:           ''
+  - key:             readability-identifier-naming.TypeAliasSuffix
+    value:           ''
+  - key:             readability-identifier-naming.TypeTemplateParameterCase
+    value:           CamelCase
+  - key:             readability-identifier-naming.TypeTemplateParameterPrefix
+    value:           ''
+  - key:             readability-identifier-naming.TypeTemplateParameterSuffix
+    value:           ''
+  - key:             readability-identifier-naming.TypedefCase
+    value:           CamelCase
+  - key:             readability-identifier-naming.TypedefPrefix
+    value:           ''
+  - key:             readability-identifier-naming.TypedefSuffix
+    value:           ''
+...
+

cpp/CMakeLists.txt

@@ -35,6 +35,9 @@ if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES)
     "Debug" "Release")
 endif()
 
+# this is needed for clang-tidy runs
+set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
+
 ##############################################################################
 # - User Options  ------------------------------------------------------------
 

cpp/bench/prims/main.cpp

@@ -14,6 +14,6 @@
  * limitations under the License.
  */
 
-#include <benchmark/benchmark.h>
+#include <benchmark/benchmark.h>  // NOLINT
 
 BENCHMARK_MAIN();

cpp/bench/sg/main.cpp

@@ -14,6 +14,6 @@
  * limitations under the License.
  */
 
-#include <benchmark/benchmark.h>
+#include <benchmark/benchmark.h>  // NOLINT
 
 BENCHMARK_MAIN();

cpp/cmake/Dependencies.cmake

@@ -219,8 +219,8 @@ set_property(TARGET benchmarklib PROPERTY
 add_dependencies(cub raft)
 add_dependencies(cutlass cub)
 add_dependencies(spdlog cutlass)
-add_dependencies(faiss spdlog)
+add_dependencies(googletest spdlog)
+add_dependencies(benchmark googletest)
+add_dependencies(faiss benchmark)
 add_dependencies(faisslib faiss)
 add_dependencies(treelite faiss)
-add_dependencies(googletest treelite)
-add_dependencies(benchmark googletest)