search

celeritas-project / celeritas

Celeritas is a new Monte Carlo transport code designed to accelerate scientific discovery in high energy physics by improving detector simulation throughput and energy efficiency using GPUs.
https://celeritas-project.github.io/celeritas/user/index.html
Other
62 stars 33 forks source link

Add initial set of clang-tidy checks #1267

Closed esseivaju closed 3 months ago

esseivaju commented 3 months ago

Add a subset of clang-tidy checks. This is not integrated with the CI (yet) as the set of enabled checks triggers many warnings but this can still be useful to run locally on individual files during development / PR review.

esseivaju commented 3 months ago

Sure, I'll add that

esseivaju commented 3 months ago

It should be ready now. Maybe at some point, if we want to require no warning, we can go through the existing warnings and either silence them with a // NOLINT comment or remove some of the checks that are too strict. For now, we can see the warnings in the build step logs.

sethrj commented 3 months ago

@esseivaju Very cool. It's a bit slow but probably worth it: I see what could be a couple of legitimate bugs or unintentional bad practices.

/home/runner/work/celeritas/celeritas/src/celeritas/optical/OpticalCollector.cc:42:9: warning: 'host_data' used after it was moved [bugprone-use-after-move]
    if (host_data.cerenkov)
        ^
/home/runner/work/celeritas/celeritas/src/celeritas/optical/OpticalCollector.cc:34:21: note: move occurred here
    storage_->obj = {std::move(host_data), inp.num_streams};
                    ^
esseivaju commented 3 months ago

yes, it is a bit slow. A solution would be to list the files changed in the PR and only check these.

esseivaju commented 3 months ago

@esseivaju Very cool. It's a bit slow but probably worth it: I see what could be a couple of legitimate bugs or unintentional bad practices.

/home/runner/work/celeritas/celeritas/src/celeritas/optical/OpticalCollector.cc:42:9: warning: 'host_data' used after it was moved [bugprone-use-after-move]
    if (host_data.cerenkov)
        ^
/home/runner/work/celeritas/celeritas/src/celeritas/optical/OpticalCollector.cc:34:21: note: move occurred here
    storage_->obj = {std::move(host_data), inp.num_streams};
                    ^

yeah, a few warnings in there should probably be fixed.