Enable more dependencies in clang-tidy checks and update (#1561)

celeritas-project · Jan 6, 2025 · f754351 · f754351
1 parent 85952d2
commit f754351
Show file tree

Hide file tree

Showing 20 changed files with 23 additions and 37 deletions.
diff --git a/.clang-tidy b/.clang-tidy
@@ -19,6 +19,7 @@ Checks: |
   -cppcoreguidelines-init-variables
   -cppcoreguidelines-pro-type-member-init
   -cppcoreguidelines-avoid-do-while
+  -cppcoreguidelines-avoid-non-const-global-variables
   -cppcoreguidelines-pro-type-const-cast
   -cppcoreguidelines-pro-bounds-array-to-pointer-decay
   -cppcoreguidelines-avoid-const-or-ref-data-members
@@ -47,7 +48,13 @@ CheckOptions:
   cppcoreguidelines-avoid-non-const-global-variables.AllowInternalLinkage: true
   performance-move-const-arg.CheckTriviallyCopyableMove: false
   performance-move-const-arg.CheckMoveToConstRef: false
-
+  bugprone-reserved-identifier.AllowedIdentifiers: |
+    R__NO_DEPRECATION;
+    G__DICTIONARY;
+    G__ROOT;
+    __TheDictionaryInitializer
+HeaderFileExtensions:         ['hh']
+ImplementationFileExtensions: ['cc']
 HeaderFilterRegex: '(app|src/(accel|celeritas|corecel|orange|geocel))/.*\.hh$'
 FormatStyle: file
 ...
diff --git a/.github/workflows/build-spack.yml b/.github/workflows/build-spack.yml
@@ -167,6 +167,14 @@ jobs:
           ALL_FILES=$(git diff --name-only --diff-filter=ACM "$BASE" HEAD)
           set +e
           CC_FILES=$(grep -E '^(src|app)/.*\.cc$' - <<< "$ALL_FILES")
+
+          # Get list of files from compile_commands.json and filter CC_FILES
+          COMPILED_FILES=$(jq -r '.[].file' build/compile_commands.json)
+          CC_FILES=$(echo "$CC_FILES" | while read -r file; do
+            if echo "$COMPILED_FILES" | grep -qE "^.*/${file}$"; then
+              echo "$file"
+            fi
+          done)
           set -e
           if [ -z "$CC_FILES" ]; then
             echo "No files to run clang-tidy on."

diff --git a/app/celer-g4/RootIO.cc b/app/celer-g4/RootIO.cc
@@ -150,6 +150,7 @@ void RootIO::WriteObject(EventData* event_data)
     }
     else
     {
+        // NOLINTNEXTLINE(bugprone-multi-level-implicit-pointer-conversion)
         event_branch_->SetAddress(&event_data);
     }
 
@@ -226,7 +227,7 @@ void RootIO::Merge()
     {
         std::string file_name = file_name_ + std::to_string(i);
         files.push_back(TFile::Open(file_name.c_str()));
-        trees.push_back((TTree*)(files[i]->Get(this->TreeName())));
+        trees.push_back(dynamic_cast<TTree*>(files[i]->Get(this->TreeName())));
         list->Add(trees[i]);
 
         if (i == nthreads - 1)

diff --git a/scripts/cmake-presets/ci-ubuntu-github.json b/scripts/cmake-presets/ci-ubuntu-github.json
@@ -94,7 +94,10 @@
       "cacheVariables": {
         "CELERITAS_DEBUG":       {"type": "BOOL", "value": "ON"},
         "CELERITAS_BUILD_TESTS": {"type": "BOOL", "value": "OFF"},
-        "CELERITAS_USE_ROOT":    {"type": "BOOL", "value": "OFF"},
+        "CELERITAS_USE_ROOT":    {"type": "BOOL", "value": "ON"},
+        "CELERITAS_USE_Perfetto":    {"type": "BOOL", "value": "OFF"},
+        "CELERITAS_USE_HepMC3":  {"type": "BOOL", "value": "ON"},
+        "CMAKE_CXX_SCAN_FOR_MODULES":  {"type": "BOOL", "value": "OFF"},
         "CMAKE_EXPORT_COMPILE_COMMANDS": {"type": "BOOL", "value": "ON"},
         "CMAKE_CXX_CLANG_TIDY": {"type": "PATH", "value": "$env{CLANG_TIDY}"}
       }

diff --git a/src/celeritas/alongstep/detail/PropagationApplier.hh b/src/celeritas/alongstep/detail/PropagationApplier.hh
@@ -112,8 +112,6 @@ PropagationApplierBaseImpl<MP>::operator()(CoreTrackView& track)
 #if CELERITAS_DEBUG
         Real3 const orig_pos = track.make_geo_view().pos();
 #endif
-        // False positive(fixed in clang-tidy-18)
-        // NOLINTNEXTLINE(clang-analyzer-core.CallAndMessage)
         auto propagate = make_propagator(track);
         p = propagate(sim.step_length());
         tracks_can_loop = propagate.tracks_can_loop();

diff --git a/src/celeritas/ext/detail/CelerOpticalPhysics.cc b/src/celeritas/ext/detail/CelerOpticalPhysics.cc
@@ -68,8 +68,6 @@ class ObservingUniquePtr
         : uptr_(std::move(ptr)), ptr_{uptr_.get()}
     {
     }
-    // False positive(fixed in clang-tidy-18)
-    // NOLINTNEXTLINE(performance-noexcept-move-constructor)
     CELER_DEFAULT_MOVE_DELETE_COPY(ObservingUniquePtr);
     ~ObservingUniquePtr() = default;
 

diff --git a/src/celeritas/user/SlotDiagnostic.cc b/src/celeritas/user/SlotDiagnostic.cc
@@ -33,6 +33,7 @@ struct SlotDiagnostic::State final : AuxStateInterface
     std::vector<int> buffer;
 
     State() = default;
+    // NOLINTNEXTLINE(performance-noexcept-move-constructor)
     CELER_DEFAULT_MOVE_DELETE_COPY(State);
     ~State() final
     {

diff --git a/src/corecel/cont/EnumArray.hh b/src/corecel/cont/EnumArray.hh
@@ -26,8 +26,6 @@ namespace celeritas
  * \todo The template parameters are reversed!!!
  */
 template<class E, class T>
-// TODO Remove in clang-tidy-18
-// NOLINTNEXTLINE(bugprone-reserved-identifier)
 struct EnumArray
 {
     static_assert(std::is_enum<E>::value, "Template parameter must be an enum");

diff --git a/src/corecel/cont/Span.hh b/src/corecel/cont/Span.hh
@@ -96,8 +96,6 @@ class Span
         : s_(other.data(), other.size())
     {
     }
-    // TODO Remove in clang-tidy-18
-    // NOLINTNEXTLINE(performance-noexcept-move-constructor)
     CELER_DEFAULT_COPY_MOVE(Span);
     ~Span() = default;
 

diff --git a/src/corecel/data/Collection.hh b/src/corecel/data/Collection.hh
@@ -276,8 +276,6 @@ class Collection
     //! Default constructors
     Collection() = default;
     Collection(Collection const&) = default;
-    // TODO Remove in clang-tidy-18
-    // NOLINTNEXTLINE(performance-noexcept-move-constructor)
     Collection(Collection&&) = default;
     //!@}
 
@@ -294,8 +292,6 @@ class Collection
     //!@{
     //! Default assignment
     Collection& operator=(Collection const& other) = default;
-    // TODO Remove in clang-tidy-18
-    // NOLINTNEXTLINE(performance-noexcept-move-constructor)
     Collection& operator=(Collection&& other) = default;
     //!@}
 

diff --git a/src/corecel/data/CollectionStateStore.hh b/src/corecel/data/CollectionStateStore.hh
@@ -77,8 +77,6 @@ class CollectionStateStore
     inline CollectionStateStore& operator=(S<W2, M2> const& other);
 
     //! Default move, delete copy (since ref "points to" val)
-    // TODO Remove in clang-tidy-18
-    // NOLINTNEXTLINE(performance-noexcept-move-constructor)
     CELER_DEFAULT_MOVE_DELETE_COPY(CollectionStateStore);
 
     //! Whether any data is being stored

diff --git a/src/corecel/data/ParamsDataInterface.hh b/src/corecel/data/ParamsDataInterface.hh
@@ -44,8 +44,6 @@ class ParamsDataInterface
 
     // Prohibit copy/move beween interface classes
     ParamsDataInterface() = default;
-    // TODO Remove in clang-tidy-18
-    // NOLINTNEXTLINE(performance-noexcept-move-constructor)
     CELER_DEFAULT_COPY_MOVE(ParamsDataInterface);
 };
 

diff --git a/src/corecel/io/StringEnumMapper.hh b/src/corecel/io/StringEnumMapper.hh
@@ -90,7 +90,6 @@ StringEnumMapper<T>::from_cstring_func(EnumCStringFuncPtr fp, char const* desc)
  */
 template<class T>
 template<class U>
-// TODO Remove in clang-tidy-18
 // NOLINTNEXTLINE(bugprone-forwarding-reference-overload)
 StringEnumMapper<T>::StringEnumMapper(U&& enum_to_string, char const* desc)
     : description_(desc)

diff --git a/src/corecel/sys/ScopedMem.cc b/src/corecel/sys/ScopedMem.cc
@@ -117,9 +117,6 @@ ScopedMem::ScopedMem(std::string_view label, MemRegistry* registry)
 /*!
  * Register data on destruction.
  */
-// bugprone-exception-escape shouldn't be needed after we upgrade to
-// clang-tidy-18
-// NOLINTNEXTLINE(bugprone-exception-escape,performance-noexcept-destructor)
 ScopedMem::~ScopedMem() noexcept(!CELERITAS_DEBUG)
 {
     if (registry_.value() != nullptr)

diff --git a/src/corecel/sys/ScopedMpiInit.cc b/src/corecel/sys/ScopedMpiInit.cc
@@ -25,8 +25,6 @@
 namespace celeritas
 {
 //---------------------------------------------------------------------------//
-// TODO Remove in clang-tidy-18
-// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 ScopedMpiInit::Status ScopedMpiInit::status_
     = ScopedMpiInit::Status::uninitialized;
 

diff --git a/src/corecel/sys/ScopedMpiInit.hh b/src/corecel/sys/ScopedMpiInit.hh
@@ -51,8 +51,6 @@ class ScopedMpiInit
 
   private:
     bool do_finalize_{false};
-    // TODO Remove in clang-tidy-18
-    // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
     static Status status_;
 };
 

diff --git a/src/geocel/rasterize/RaytraceImager.hh b/src/geocel/rasterize/RaytraceImager.hh
@@ -70,8 +70,6 @@ class RaytraceImager final : public ImagerInterface
 
     //// MEMBER FUNCTIONS ////
 
-    // TODO Remove in clang-tidy-18
-    // NOLINTNEXTLINE(performance-noexcept-move-constructor)
     CELER_DEFAULT_MOVE_DELETE_COPY(RaytraceImager);
 
     template<MemSpace M>

diff --git a/src/orange/orangeinp/InputBuilder.cc b/src/orange/orangeinp/InputBuilder.cc
@@ -51,8 +51,6 @@ void write_protos(detail::ProtoMap const& map, std::string const& filename)
 class JsonProtoOutput
 {
   public:
-    // false positive with clang-tidy-15, need cleanup
-    // NOLINTNEXTLINE(bugprone-exception-escape)
     JsonProtoOutput() = default;
 
     //! Construct with the number of universes

diff --git a/src/orange/surf/GeneralQuadric.cc b/src/orange/surf/GeneralQuadric.cc
@@ -43,8 +43,6 @@ GeneralQuadric::GeneralQuadric(Real3 const& abc,
 /*!
  * Promote from a simple quadric.
  */
-// Remove after upgrade to clang-tidy-18:
-// NOLINTNEXTLINE(bugprone-exception-escape)
 GeneralQuadric::GeneralQuadric(SimpleQuadric const& other) noexcept(
     !CELERITAS_DEBUG)
     : GeneralQuadric{make_array(other.second()),

diff --git a/src/orange/surf/SimpleQuadric.cc b/src/orange/surf/SimpleQuadric.cc
@@ -47,8 +47,6 @@ SimpleQuadric::SimpleQuadric(Real3 const& abc, Real3 const& def, real_type g)
  * whereas the simple quadric has a different sign for the constant:
  * \f$ dx + ey + fz + g = 0 \f$ .
  */
-// Remove after upgrade to clang-tidy-18:
-// NOLINTNEXTLINE(bugprone-exception-escape)
 SimpleQuadric::SimpleQuadric(Plane const& other) noexcept(!CELERITAS_DEBUG)
     : SimpleQuadric{{0, 0, 0}, other.normal(), negate(other.displacement())}
 {
@@ -95,8 +93,6 @@ template SimpleQuadric::SimpleQuadric(CylAligned<Axis::z> const&) noexcept;
        + x_0^2 + y_0^2 + z_0^2 - r^2 = 0
  * \endverbatim
  */
-// Remove after upgrade to clang-tidy-18:
-// NOLINTNEXTLINE(bugprone-exception-escape)
 SimpleQuadric::SimpleQuadric(Sphere const& other) noexcept(!CELERITAS_DEBUG)
 {
     Real3 const& origin = other.origin();