Closed bcaddy closed 1 year ago
This PR is pretty much done. I've added clang-tidy documentation to the Making a Pull Request section of the wiki. All that's left is:
.git-blame-rev-ignore-revs
@ojwg, I had to make some minor tweaks to feedback_CIC_gpu.cu to get it to pass clang-tidy. Could you review them to make sure they're ok?
@evaneschneider, Once @ojwg's review is done this is ready to merge
Clang-Format
Clang-format can be run in a variety of ways.
tools/clang-format_runner.sh
scriptmake format
Tested with
clang-format
15Added the following
.clang-format
file with the formatting we decided on based off of Google's format.git-blame-ignore-revs
file to help us ignore formatting changes, see that file for how to implement it for you.git-blame-ignore-revs
tools/clang-format_runner.sh
script to runclang-format
on all the C++ source files in Chollacode_formatting.yml
GitHub Actions workflow automatically check formatting on each push and PRClang-Tidy
clang-tidy
can be run by runningmake tidy
. Tested withclang-tidy
15..clang-tidy
filetools/analyze_tidy_checks.py
script that prints a summary of the failed clang-tidy checks and how many times each check failedDocker
Other
-foffload=disable
as it's no longer usedStanding Issues
Here's some outstanding issues we need to address before this can be merged. I've tagged the people I think are most relevant for each issue, if I get anything wrong please let me know and I will correct it.
.git-blame-ignore-revs
(@bcaddy)clang-tidy
currently doesn't pass on all builds as it throws severalclang-diagnostic-error
errors. These are errors from the clang compiler, notclang-tidy
and so cannot be turned off. I believe all the errors will be easy to fix, and I already fixed one, but some of them are in areas of the code I haven't touched and so I'd appreciate it if others looked into it. Here's a list of the current errors, all are on ROCm builds:/cholla/cholla/src/gravity/paris/HenryPeriodic.cu:85:42: error: use of undeclared identifier 'cudaHostAllocDefault' [clang-diagnostic-error]
- Either helping it find the macro or replace with a standard allocation (or std::vector)/cholla/cholla/src/particles/feedback_CIC_gpu.cu:382:32: error: no matching function for call to 'atomicMax' [clang-diagnostic-error]
- (@ojwg)/opt/rocm-5.2.3/include/hiprand/hiprand_kernel_nvcc.h:43:1: error: typedef redefinition with different types ('struct hiprandStateMRG32k3a' vs 'struct curandStateMRG32k3a') [clang-diagnostic-error]
- I'm not sure how to deal with this one since it's not in our code. Maybe @twhite-cray or Damon have some ideas.