search

rapidsai / legate-boost

GBM implementation on Legate
https://rapidsai.github.io/legate-boost/
Apache License 2.0
8 stars 8 forks source link

Integrate static anaylsis of C++ code base into CI #184

Open RAMitchell opened 5 days ago

RAMitchell commented 5 days ago

Use clang-tidy or another static analysis tool to increase the safety/reliability of our C++ codebase.

jameslamb commented 5 days ago

Just dumping some other tool names here, for whenever this is picked up.

And others that require actually compiling and running (so not static analysis, but maybe still helpful within the spirit of this issue):

RAMitchell commented 4 days ago

A couple of options I have tried:

cpplint can be integrated with pre-commit and works nicely out of the box, even with cuda code. Its warnings are pretty superficial and mostly style related but some things like missing copyright are useful.

clang-tidy gives really nice warnings particularly in modernisation (use C++11 features) and applying the C++ core guidelines for good coding practices. It is very difficult to integrate into a non-clang build with cuda code. Xgboost runs it with this script , but I find this to be very fragile and hacky.

RAMitchell commented 4 days ago

cppcheck has issues with macros in host code lambdas, throwing syntax errors. 

src/cpp_utils/cpp_utils.h:255:45: error: syntax error [syntaxError]
        policy, UnravelIter(in_shape), v, [=] __host__ __device__(const legate::Point<kDim>& p) {

Otherwise it did not detect any issues in the c++ files unless I insert something obvious like

char a[10];
a[10] = 0;

As it can't read cuda files and because of the above issue I will probably skip cppcheck.