chore: Update compiler warning flags (#3495)

- Remove `-Wno-unused-local-typedefs`
- Add `-Wzero-as-null-pointer-constant`, `-Wold-style-cast` and `-Wnull-derefence`
- Fix warnings caused by these flags

- [x] This requires an update to ODD, since ODD triggers old-style cast warnings currently.
- [x] Depends on acts-project/actsvg#80 to avoid warnings from ACTSVG

Blocked by
-  #3509

.pre-commit-config.yaml

@@ -10,7 +10,9 @@ repos:
     rev: v3.2.0
     hooks:
     - id: trailing-whitespace
+      exclude: \.(diff|patch)$
     - id: end-of-file-fixer
+      exclude: \.(diff|patch)$
     - id: check-yaml
     - id: check-added-large-files
 

Core/include/Acts/Definitions/Algebra.hpp

@@ -12,7 +12,7 @@
 #if defined(__GNUC__) && !defined(__clang__) && !defined(__INTEL_COMPILER)
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wmisleading-indentation"
-#if __GNUC__ == 13
+#if __GNUC__ >= 12
 #pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
 #endif
 #include <Eigen/Core>

Core/include/Acts/Propagator/DenseEnvironmentExtension.hpp

@@ -124,7 +124,12 @@ struct DenseEnvironmentExtension {
     // i = 0 is used for setup and evaluation of k
     if (i == 0) {
       // Set up container for energy loss
-      auto volumeMaterial = navigator.currentVolumeMaterial(state.navigation);
+      const auto* volumeMaterial =
+          navigator.currentVolumeMaterial(state.navigation);
+      if (volumeMaterial == nullptr) {
+        // This function is very hot, so we prefer to terminate here
+        std::terminate();
+      }
       ThisVector3 position = stepper.position(state.stepping);
       material = volumeMaterial->material(position.template cast<double>());
       initialMomentum = stepper.absoluteMomentum(state.stepping);

Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp

@@ -396,11 +396,11 @@ class MultiEigenStepperLoop
   /// proxy internally holding a reference
   auto componentIterable(State& state) const {
     struct Iterator {
-      using difference_type = std::ptrdiff_t;
-      using value_type = ComponentProxy;
-      using reference = ComponentProxy;
-      using pointer = void;
-      using iterator_category = std::forward_iterator_tag;
+      using difference_type [[maybe_unused]] = std::ptrdiff_t;
+      using value_type [[maybe_unused]] = ComponentProxy;
+      using reference [[maybe_unused]] = ComponentProxy;
+      using pointer [[maybe_unused]] = void;
+      using iterator_category [[maybe_unused]] = std::forward_iterator_tag;
 
       typename decltype(state.components)::iterator it;
       const State& s;
@@ -414,8 +414,6 @@ class MultiEigenStepperLoop
     };
 
     struct Iterable {
-      using iterator = Iterator;
-
       State& s;
 
       // clang-format off
@@ -433,11 +431,11 @@ class MultiEigenStepperLoop
   /// proxy internally holding a reference
   auto constComponentIterable(const State& state) const {
     struct ConstIterator {
-      using difference_type = std::ptrdiff_t;
-      using value_type = ConstComponentProxy;
-      using reference = ConstComponentProxy;
-      using pointer = void;
-      using iterator_category = std::forward_iterator_tag;
+      using difference_type [[maybe_unused]] = std::ptrdiff_t;
+      using value_type [[maybe_unused]] = ConstComponentProxy;
+      using reference [[maybe_unused]] = ConstComponentProxy;
+      using pointer [[maybe_unused]] = void;
+      using iterator_category [[maybe_unused]] = std::forward_iterator_tag;
 
       typename decltype(state.components)::const_iterator it;
       const State& s;
@@ -451,7 +449,6 @@ class MultiEigenStepperLoop
     };
 
     struct Iterable {
-      using iterator = ConstIterator;
       const State& s;
 
       // clang-format off

Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp

@@ -1227,7 +1227,6 @@ class CombinatorialKalmanFilter {
                   track_container_t& trackContainer) const
       -> Result<std::vector<
           typename std::decay_t<decltype(trackContainer)>::TrackProxy>> {
-    using TrackContainer = typename std::decay_t<decltype(trackContainer)>;
     using SourceLinkAccessor =
         SourceLinkAccessorDelegate<source_link_iterator_t>;
 

Core/include/Acts/TrackFitting/GaussianSumFitter.hpp

@@ -349,8 +349,6 @@ struct GaussianSumFitter {
                                   ? *options.referenceSurface
                                   : sParameters.referenceSurface();
 
-      using PM = TrackStatePropMask;
-
       const auto& params = *fwdGsfResult.lastMeasurementState;
       auto state =
           m_propagator.template makeState<MultiComponentBoundTrackParameters,

Core/include/Acts/TrackFitting/detail/GsfActor.hpp

@@ -188,7 +188,7 @@ struct GsfActor {
     // All components must have status "on surface". It is however possible,
     // that currentSurface is nullptr and all components are "on surface" (e.g.,
     // for surfaces excluded from the navigation)
-    using Status = Acts::Intersection3D::Status;
+    using Status [[maybe_unused]] = Acts::Intersection3D::Status;
     assert(std::all_of(
         stepperComponents.begin(), stepperComponents.end(),
         [](const auto& cmp) { return cmp.status() == Status::onSurface; }));

Core/include/Acts/Utilities/Any.hpp

@@ -124,8 +124,9 @@ class AnyBase : public AnyBaseAll {
     if constexpr (!heapAllocated<U>()) {
       // construct into local buffer
       /*U* ptr =*/new (m_data.data()) U(std::forward<Args>(args)...);
-      _ACTS_ANY_VERBOSE(
-          "Construct local (this=" << this << ") at: " << (void*)m_data.data());
+      _ACTS_ANY_VERBOSE("Construct local (this="
+                        << this
+                        << ") at: " << static_cast<void*>(m_data.data()));
     } else {
       // too large, heap allocate
       U* heap = new U(std::forward<Args>(args)...);
@@ -181,16 +182,16 @@ class AnyBase : public AnyBaseAll {
       return;
     }
 
-    _ACTS_ANY_VERBOSE(
-        "Copy construct (this=" << this << ") at: " << (void*)m_data.data());
+    _ACTS_ANY_VERBOSE("Copy construct (this="
+                      << this << ") at: " << static_cast<void*>(m_data.data()));
 
     m_handler = other.m_handler;
     copyConstruct(other);
   }
 
   AnyBase& operator=(const AnyBase& other) {
-    _ACTS_ANY_VERBOSE("Copy assign (this=" << this
-                                           << ") at: " << (void*)m_data.data());
+    _ACTS_ANY_VERBOSE("Copy assign (this="
+                      << this << ") at: " << static_cast<void*>(m_data.data()));
 
     if (m_handler == nullptr && other.m_handler == nullptr) {
       // both are empty, noop
@@ -213,8 +214,8 @@ class AnyBase : public AnyBaseAll {
   }
 
   AnyBase(AnyBase&& other) {
-    _ACTS_ANY_VERBOSE(
-        "Move construct (this=" << this << ") at: " << (void*)m_data.data());
+    _ACTS_ANY_VERBOSE("Move construct (this="
+                      << this << ") at: " << static_cast<void*>(m_data.data()));
     if (m_handler == nullptr && other.m_handler == nullptr) {
       // both are empty, noop
       return;
@@ -225,8 +226,8 @@ class AnyBase : public AnyBaseAll {
   }
 
   AnyBase& operator=(AnyBase&& other) {
-    _ACTS_ANY_VERBOSE("Move assign (this=" << this
-                                           << ") at: " << (void*)m_data.data());
+    _ACTS_ANY_VERBOSE("Move assign (this="
+                      << this << ") at: " << static_cast<void*>(m_data.data()));
     if (m_handler == nullptr && other.m_handler == nullptr) {
       // both are empty, noop
       return *this;

Core/include/Acts/Utilities/GridBinFinder.hpp

@@ -29,6 +29,7 @@ namespace Acts {
 template <std::size_t DIM>
 class GridBinFinder {
  public:
+  static constexpr std::size_t dimCubed = Acts::detail::ipow(3, DIM);
   /// @brief Constructor
   /// @tparam args ... Input parameters provided by the user
   ///
@@ -63,9 +64,9 @@ class GridBinFinder {
   ///
   /// @pre The provided local position must be a valid local bins configuration in the grid
   template <typename stored_t, class... Axes>
-  boost::container::small_vector<std::size_t, Acts::detail::ipow(3, DIM)>
-  findBins(const std::array<std::size_t, DIM>& locPosition,
-           const Acts::Grid<stored_t, Axes...>& grid) const;
+  boost::container::small_vector<std::size_t, dimCubed> findBins(
+      const std::array<std::size_t, DIM>& locPosition,
+      const Acts::Grid<stored_t, Axes...>& grid) const;
 
  private:
   /// @brief Store the values provided by the user for each axis in the grid

Core/include/Acts/Utilities/GridBinFinder.ipp

@@ -62,10 +62,10 @@ std::array<std::pair<int, int>, DIM> Acts::GridBinFinder<DIM>::getSizePerAxis(
 
 template <std::size_t DIM>
 template <typename stored_t, class... Axes>
-boost::container::small_vector<std::size_t, Acts::detail::ipow(3, DIM)>
-Acts::GridBinFinder<DIM>::findBins(
+auto Acts::GridBinFinder<DIM>::findBins(
     const std::array<std::size_t, DIM>& locPosition,
-    const Acts::Grid<stored_t, Axes...>& grid) const {
+    const Acts::Grid<stored_t, Axes...>& grid) const
+    -> boost::container::small_vector<std::size_t, dimCubed> {
   static_assert(sizeof...(Axes) == DIM);
   assert(isGridCompatible(grid));
   std::array<std::pair<int, int>, DIM> sizePerAxis =

Core/src/Material/SurfaceMaterialMapper.cpp

@@ -286,8 +286,6 @@ void Acts::SurfaceMaterialMapper::mapInteraction(
   // To remember the bins of this event
   using MapBin =
       std::pair<AccumulatedSurfaceMaterial*, std::array<std::size_t, 3>>;
-  using MaterialBin = std::pair<AccumulatedSurfaceMaterial*,
-                                std::shared_ptr<const ISurfaceMaterial>>;
   std::map<AccumulatedSurfaceMaterial*, std::array<std::size_t, 3>>
       touchedMapBins;
   std::map<AccumulatedSurfaceMaterial*, std::shared_ptr<const ISurfaceMaterial>>

Examples/Algorithms/Geant4/src/SensitiveSteppingAction.cpp

@@ -131,6 +131,10 @@ void ActsExamples::SensitiveSteppingAction::UserSteppingAction(
 
   // Get the physical volume & check if it has the sensitive string name
   const G4VPhysicalVolume* volume = track->GetVolume();
+  if (volume == nullptr) {
+    ACTS_ERROR("No volume found for track " << track->GetTrackID());
+    std::terminate();
+  }
   std::string volumeName = volume->GetName();
 
   if (volumeName.find(SensitiveSurfaceMapper::mappingPrefix) ==

Examples/Detectors/ContextualDetector/src/ExternalAlignmentDecorator.cpp

@@ -110,9 +110,15 @@ void ActsExamples::Contextual::ExternalAlignmentDecorator::parseGeometry(
                                        Acts::Transform3::Identity());
 
   auto fillTransforms = [&aStore, &nominalCtx](const auto* surface) -> void {
+    if (surface == nullptr) {
+      throw std::invalid_argument("Surface is nullptr.");
+    }
     auto alignableElement =
         dynamic_cast<const ExternallyAlignedDetectorElement*>(
             surface->associatedDetectorElement());
+    if (alignableElement == nullptr) {
+      throw std::invalid_argument("Surface is not alignable");
+    }
     aStore[alignableElement->identifier()] = surface->transform(nominalCtx);
   };
 

Examples/Framework/src/EventData/ScalingCalibrator.cpp

@@ -85,7 +85,7 @@ readMaps(const std::filesystem::path& path) {
 
   for (auto it = lst->begin(); it != lst->end(); ++it) {
     TKey* key = static_cast<TKey*>(*it);
-    if (std::strcmp(key->GetClassName(), "TH2D") == 0) {
+    if (key != nullptr && std::strcmp(key->GetClassName(), "TH2D") == 0) {
       auto [geoId, var] = parseMapKey(key->GetName());
 
       TH2D hist;

Examples/Io/EDM4hep/src/EDM4hepReader.cpp

@@ -250,8 +250,13 @@ ProcessCode EDM4hepReader::read(const AlgorithmContext& ctx) {
   SimParticleContainer particlesFinal;
   SimParticleContainer particlesGenerator;
   for (const auto& inParticle : mcParticleCollection) {
-    const std::size_t index =
-        edm4hepParticleMap.find(inParticle.getObjectID().index)->second;
+    auto particleIt = edm4hepParticleMap.find(inParticle.getObjectID().index);
+    if (particleIt == edm4hepParticleMap.end()) {
+      ACTS_ERROR("Particle " << inParticle.getObjectID().index
+                             << " not found in particle map");
+      continue;
+    }
+    const std::size_t index = particleIt->second;
     const auto& particleInitial = unordered.at(index);
     if (!inParticle.isCreatedInSimulation()) {
       particlesGenerator.insert(particleInitial);
@@ -348,8 +353,14 @@ ProcessCode EDM4hepReader::read(const AlgorithmContext& ctx) {
             if (it == m_surfaceMap.end()) {
               ACTS_ERROR("Unable to find surface for detElement "
                          << detElement.name() << " with cellId " << cellId);
+              throw std::runtime_error("Unable to find surface for detElement");
             }
             const auto* surface = it->second;
+            if (surface == nullptr) {
+              ACTS_ERROR("Unable to find surface for detElement "
+                         << detElement.name() << " with cellId " << cellId);
+              throw std::runtime_error("Unable to find surface for detElement");
+            }
             ACTS_VERBOSE("   -> surface: " << surface->geometryId());
             return surface->geometryId();
           });

Examples/Io/Performance/ActsExamples/Io/Performance/CKFPerformanceWriter.cpp

@@ -150,7 +150,6 @@ ProcessCode CKFPerformanceWriter::finalize() {
 ProcessCode CKFPerformanceWriter::writeT(const AlgorithmContext& ctx,
                                          const ConstTrackContainer& tracks) {
   // The number of majority particle hits and fitted track parameters
-  using RecoTrackInfo = std::pair<std::size_t, Acts::BoundTrackParameters>;
   using Acts::VectorHelpers::perp;
 
   // Read truth input collections

Examples/Io/Root/CMakeLists.txt

@@ -44,5 +44,6 @@ root_generate_dictionary(
   ActsExamplesIoRootDict MODULE ActsExamplesIoRoot LINKDEF LinkDef.hpp
 )
 set_target_properties(ActsExamplesIoRootDict PROPERTIES CXX_CLANG_TIDY "")
+target_compile_options(ActsExamplesIoRootDict PRIVATE "-Wno-old-style-cast")
 
 install(TARGETS ActsExamplesIoRoot LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR})

Examples/Scripts/MaterialMapping/materialComposition.C

@@ -43,10 +43,10 @@ struct MaterialHistograms {
         (iA == 0) ? name + std::string("_l0_vs_eta_all")
                   : name + std::string("_l0_vs_eta_A") + std::to_string(iA);
 
-    x0_vs_eta = new TProfile(x0NameEta.c_str(), "X_{0} vs. #eta", bins, -eta,
-                             eta);
-    l0_vs_eta = new TProfile(l0NameEta.c_str(), "L_{0} vs. #eta", bins, -eta,
-                             eta);
+    x0_vs_eta =
+        new TProfile(x0NameEta.c_str(), "X_{0} vs. #eta", bins, -eta, eta);
+    l0_vs_eta =
+        new TProfile(l0NameEta.c_str(), "L_{0} vs. #eta", bins, -eta, eta);
 
     std::string x0NamePhi =
         (iA == 0) ? name + std::string("_x0_vs_phi_all")
@@ -55,10 +55,10 @@ struct MaterialHistograms {
         (iA == 0) ? name + std::string("_l0_vs_phi_all")
                   : name + std::string("_l0_vs_phi_A") + std::to_string(iA);
 
-    x0_vs_phi = new TProfile(x0NamePhi.c_str(), "X_{0} vs. #phi", bins, -M_PI,
-                             M_PI);
-    l0_vs_phi = new TProfile(l0NamePhi.c_str(), "L_{0} vs. #phi", bins, -M_PI,
-                             M_PI);
+    x0_vs_phi =
+        new TProfile(x0NamePhi.c_str(), "X_{0} vs. #phi", bins, -M_PI, M_PI);
+    l0_vs_phi =
+        new TProfile(l0NamePhi.c_str(), "L_{0} vs. #phi", bins, -M_PI, M_PI);
   }
 
   /// This fills the event into the histograms
@@ -132,6 +132,10 @@ void materialComposition(const std::string& inFile, const std::string& treeName,
   // Draw all the atomic elements & get the histogram
   inputTree->Draw("mat_A>>hA(200,0.5,200.5)");
   TH1F* histA = dynamic_cast<TH1F*>(gDirectory->Get("hA"));
+  if (histA == nullptr) {
+    throw std::runtime_error{"Could not get the histogram"};
+  }
+
   histA->Draw();
 
   auto outputFile = TFile::Open(outFile.c_str(), "recreate");

Plugins/DD4hep/src/DD4hepLayerBuilder.cpp

@@ -128,6 +128,8 @@ const Acts::LayerVector Acts::DD4hepLayerBuilder::endcapLayers(
         TGeoTubeSeg* tube = dynamic_cast<TGeoTubeSeg*>(geoShape);
         if (tube == nullptr) {
           ACTS_ERROR(" Disc layer has wrong shape - needs to be TGeoTubeSeg!");
+          throw std::logic_error{
+              "Disc layer has wrong shape - needs to be TGeoTubeSeg!"};
         }
         // extract the boundaries
         double rMin = tube->GetRmin() * UnitConstants::cm;
@@ -299,6 +301,8 @@ const Acts::LayerVector Acts::DD4hepLayerBuilder::centralLayers(
         if (tube == nullptr) {
           ACTS_ERROR(
               " Cylinder layer has wrong shape - needs to be TGeoTubeSeg!");
+          throw std::logic_error{
+              " Cylinder layer has wrong shape - needs to be TGeoTubeSeg!"};
         }
 
         // extract the boundaries

Plugins/DD4hep/src/DD4hepVolumeBuilder.cpp

@@ -64,6 +64,8 @@ Acts::DD4hepVolumeBuilder::centralVolumes() const {
       if (tube == nullptr) {
         ACTS_ERROR(
             "[L] Cylinder layer has wrong shape - needs to be TGeoTubeSeg!");
+        throw std::logic_error{
+            "[L] Cylinder layer has wrong shape - needs to be TGeoTubeSeg!"};
       }
 
       // Extract the boundaries

Plugins/Detray/src/DetrayConverter.cpp

@@ -79,9 +79,6 @@ detray::io::mask_payload Acts::DetrayConverter::convertMask(
 
 detray::io::surface_payload Acts::DetrayConverter::convertSurface(
     const Acts::GeometryContext& gctx, const Surface& surface, bool portal) {
-  using material_link_payload =
-      detray::io::typed_link_payload<detray::io::material_id>;
-
   detray::io::surface_payload surfacePayload;
 
   surfacePayload.transform = convertTransform(surface.transform(gctx));
@@ -176,13 +173,13 @@ std::vector<detray::io::surface_payload> Acts::DetrayConverter::convertPortal(
       std::array<ActsScalar, 2u> clipRange = {0., 0.};
       std::vector<ActsScalar> boundValues = surfaceAdjusted->bounds().values();
       if (surfaceType == Surface::SurfaceType::Cylinder &&
-          cast == Acts::BinningValue::binZ) {
+          cast == BinningValue::binZ) {
         ActsScalar zPosition = surfaceAdjusted->center(gctx).z();
         clipRange = {
             zPosition - boundValues[CylinderBounds::BoundValues::eHalfLengthZ],
             zPosition + boundValues[CylinderBounds::BoundValues::eHalfLengthZ]};
       } else if (surfaceType == Surface::SurfaceType::Disc &&
-                 cast == Acts::BinningValue::binR) {
+                 cast == BinningValue::binR) {
         clipRange = {boundValues[RadialBounds::BoundValues::eMinR],
                      boundValues[RadialBounds::BoundValues::eMaxR]};
       } else {

Plugins/Podio/CMakeLists.txt

@@ -27,9 +27,13 @@ target_link_libraries(
     PUBLIC ActsPodioEdm ROOT::Core podio::podio podio::podioRootIO
 )
 
+target_compile_options(ActsPodioEdm PRIVATE "-Wno-old-style-cast")
+
 podio_add_root_io_dict(ActsPodioEdmDict ActsPodioEdm "${headers}" src/selection.xml)
 add_library(Acts::ActsPodioEdmDict ALIAS ActsPodioEdmDict)
 
+target_compile_options(ActsPodioEdmDict PRIVATE "-Wno-old-style-cast")
+
 install(
     TARGETS ActsPodioEdm
     EXPORT ActsPodioEdmTargets