From 77d1f88947a7f7bf4da082ad81ccdb5fac3bda88 Mon Sep 17 00:00:00 2001 From: Paul Gessinger Date: Wed, 21 Aug 2024 00:06:19 +0200 Subject: [PATCH] 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 https://github.com/acts-project/actsvg/pull/80 to avoid warnings from ACTSVG Blocked by - #3509 --- .pre-commit-config.yaml | 2 + Core/include/Acts/Definitions/Algebra.hpp | 2 +- .../Propagator/DenseEnvironmentExtension.hpp | 7 +++- .../Acts/Propagator/MultiEigenStepperLoop.hpp | 23 +++++------ .../CombinatorialKalmanFilter.hpp | 1 - .../Acts/TrackFitting/GaussianSumFitter.hpp | 2 - .../Acts/TrackFitting/detail/GsfActor.hpp | 2 +- Core/include/Acts/Utilities/Any.hpp | 21 +++++----- Core/include/Acts/Utilities/GridBinFinder.hpp | 7 ++-- Core/include/Acts/Utilities/GridBinFinder.ipp | 6 +-- Core/src/Material/SurfaceMaterialMapper.cpp | 2 - .../Geant4/src/SensitiveSteppingAction.cpp | 4 ++ .../src/ExternalAlignmentDecorator.cpp | 6 +++ .../src/EventData/ScalingCalibrator.cpp | 2 +- Examples/Io/EDM4hep/src/EDM4hepReader.cpp | 15 ++++++- .../Io/Performance/CKFPerformanceWriter.cpp | 1 - Examples/Io/Root/CMakeLists.txt | 1 + .../MaterialMapping/materialComposition.C | 20 ++++++---- Plugins/DD4hep/src/DD4hepLayerBuilder.cpp | 4 ++ Plugins/DD4hep/src/DD4hepVolumeBuilder.cpp | 2 + Plugins/Detray/src/DetrayConverter.cpp | 7 +--- Plugins/Podio/CMakeLists.txt | 4 ++ .../Plugins/Podio/PodioTrackContainer.hpp | 4 ++ .../Podio/PodioTrackStateContainer.hpp | 6 ++- Plugins/Podio/src/PodioUtil.cpp | 3 -- Plugins/TGeo/src/TGeoParser.cpp | 35 ++++++++-------- .../Geometry/CylinderVolumeStackTests.cpp | 2 +- .../Material/GridSurfaceMaterialTests.cpp | 2 - .../MaterialInteractionAssignmentTests.cpp | 2 - .../Core/Propagator/MultiStepperTests.cpp | 4 +- .../Core/Vertexing/ZScanVertexFinderTests.cpp | 4 -- .../EDM4hep/ConvertTrackEDM4hepTest.cpp | 1 - cmake/ActsCompilerOptions.cmake | 16 +++++++- thirdparty/OpenDataDetector | 2 +- thirdparty/traccc/CMakeLists.txt | 12 +++++- thirdparty/traccc/warnings.diff | 40 +++++++++++++++++++ 36 files changed, 185 insertions(+), 89 deletions(-) create mode 100644 thirdparty/traccc/warnings.diff diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b8941ba3802..9000c17f6e9 100644 --- a/.pre-commit-config.yaml +++ b/.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 diff --git a/Core/include/Acts/Definitions/Algebra.hpp b/Core/include/Acts/Definitions/Algebra.hpp index a8b54653d23..fa5edcc2252 100644 --- a/Core/include/Acts/Definitions/Algebra.hpp +++ b/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 diff --git a/Core/include/Acts/Propagator/DenseEnvironmentExtension.hpp b/Core/include/Acts/Propagator/DenseEnvironmentExtension.hpp index 68868de8a92..eed51e1e847 100644 --- a/Core/include/Acts/Propagator/DenseEnvironmentExtension.hpp +++ b/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()); initialMomentum = stepper.absoluteMomentum(state.stepping); diff --git a/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp b/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp index 41e68890c94..5fe9d0c06d7 100644 --- a/Core/include/Acts/Propagator/MultiEigenStepperLoop.hpp +++ b/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 diff --git a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp index 9231ed580d9..51730f33adf 100644 --- a/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp +++ b/Core/include/Acts/TrackFinding/CombinatorialKalmanFilter.hpp @@ -1227,7 +1227,6 @@ class CombinatorialKalmanFilter { track_container_t& trackContainer) const -> Result::TrackProxy>> { - using TrackContainer = typename std::decay_t; using SourceLinkAccessor = SourceLinkAccessorDelegate; diff --git a/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp b/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp index cd5cacce204..0b1f9d5e78d 100644 --- a/Core/include/Acts/TrackFitting/GaussianSumFitter.hpp +++ b/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()) { // construct into local buffer /*U* ptr =*/new (m_data.data()) U(std::forward(args)...); - _ACTS_ANY_VERBOSE( - "Construct local (this=" << this << ") at: " << (void*)m_data.data()); + _ACTS_ANY_VERBOSE("Construct local (this=" + << this + << ") at: " << static_cast(m_data.data())); } else { // too large, heap allocate U* heap = new U(std::forward(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(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(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(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(m_data.data())); if (m_handler == nullptr && other.m_handler == nullptr) { // both are empty, noop return *this; diff --git a/Core/include/Acts/Utilities/GridBinFinder.hpp b/Core/include/Acts/Utilities/GridBinFinder.hpp index 9d57790459d..da2f9ecbfde 100644 --- a/Core/include/Acts/Utilities/GridBinFinder.hpp +++ b/Core/include/Acts/Utilities/GridBinFinder.hpp @@ -29,6 +29,7 @@ namespace Acts { template 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 - boost::container::small_vector - findBins(const std::array& locPosition, - const Acts::Grid& grid) const; + boost::container::small_vector findBins( + const std::array& locPosition, + const Acts::Grid& grid) const; private: /// @brief Store the values provided by the user for each axis in the grid diff --git a/Core/include/Acts/Utilities/GridBinFinder.ipp b/Core/include/Acts/Utilities/GridBinFinder.ipp index 082a8efc475..344ef07137c 100644 --- a/Core/include/Acts/Utilities/GridBinFinder.ipp +++ b/Core/include/Acts/Utilities/GridBinFinder.ipp @@ -62,10 +62,10 @@ std::array, DIM> Acts::GridBinFinder::getSizePerAxis( template template -boost::container::small_vector -Acts::GridBinFinder::findBins( +auto Acts::GridBinFinder::findBins( const std::array& locPosition, - const Acts::Grid& grid) const { + const Acts::Grid& grid) const + -> boost::container::small_vector { static_assert(sizeof...(Axes) == DIM); assert(isGridCompatible(grid)); std::array, DIM> sizePerAxis = diff --git a/Core/src/Material/SurfaceMaterialMapper.cpp b/Core/src/Material/SurfaceMaterialMapper.cpp index 1a5ae1ba382..a8d62f0e75c 100644 --- a/Core/src/Material/SurfaceMaterialMapper.cpp +++ b/Core/src/Material/SurfaceMaterialMapper.cpp @@ -286,8 +286,6 @@ void Acts::SurfaceMaterialMapper::mapInteraction( // To remember the bins of this event using MapBin = std::pair>; - using MaterialBin = std::pair>; std::map> touchedMapBins; std::map> diff --git a/Examples/Algorithms/Geant4/src/SensitiveSteppingAction.cpp b/Examples/Algorithms/Geant4/src/SensitiveSteppingAction.cpp index f0537ad7226..ec07dc5f3a6 100644 --- a/Examples/Algorithms/Geant4/src/SensitiveSteppingAction.cpp +++ b/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) == diff --git a/Examples/Detectors/ContextualDetector/src/ExternalAlignmentDecorator.cpp b/Examples/Detectors/ContextualDetector/src/ExternalAlignmentDecorator.cpp index 30f5a6072ce..065ef4c6e2f 100644 --- a/Examples/Detectors/ContextualDetector/src/ExternalAlignmentDecorator.cpp +++ b/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( surface->associatedDetectorElement()); + if (alignableElement == nullptr) { + throw std::invalid_argument("Surface is not alignable"); + } aStore[alignableElement->identifier()] = surface->transform(nominalCtx); }; diff --git a/Examples/Framework/src/EventData/ScalingCalibrator.cpp b/Examples/Framework/src/EventData/ScalingCalibrator.cpp index 6681af67ee4..805d8cdef15 100644 --- a/Examples/Framework/src/EventData/ScalingCalibrator.cpp +++ b/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(*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; diff --git a/Examples/Io/EDM4hep/src/EDM4hepReader.cpp b/Examples/Io/EDM4hep/src/EDM4hepReader.cpp index 34cbf1e345a..7e61ba2a561 100644 --- a/Examples/Io/EDM4hep/src/EDM4hepReader.cpp +++ b/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(); }); diff --git a/Examples/Io/Performance/ActsExamples/Io/Performance/CKFPerformanceWriter.cpp b/Examples/Io/Performance/ActsExamples/Io/Performance/CKFPerformanceWriter.cpp index 107a331f2e6..3caad01b002 100644 --- a/Examples/Io/Performance/ActsExamples/Io/Performance/CKFPerformanceWriter.cpp +++ b/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; using Acts::VectorHelpers::perp; // Read truth input collections diff --git a/Examples/Io/Root/CMakeLists.txt b/Examples/Io/Root/CMakeLists.txt index 3823be24194..694294c3c8f 100644 --- a/Examples/Io/Root/CMakeLists.txt +++ b/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}) diff --git a/Examples/Scripts/MaterialMapping/materialComposition.C b/Examples/Scripts/MaterialMapping/materialComposition.C index 40bfd2b8a57..ecd3863f19d 100644 --- a/Examples/Scripts/MaterialMapping/materialComposition.C +++ b/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(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"); diff --git a/Plugins/DD4hep/src/DD4hepLayerBuilder.cpp b/Plugins/DD4hep/src/DD4hepLayerBuilder.cpp index 2fb52291a04..4d604fbeb42 100644 --- a/Plugins/DD4hep/src/DD4hepLayerBuilder.cpp +++ b/Plugins/DD4hep/src/DD4hepLayerBuilder.cpp @@ -128,6 +128,8 @@ const Acts::LayerVector Acts::DD4hepLayerBuilder::endcapLayers( TGeoTubeSeg* tube = dynamic_cast(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 diff --git a/Plugins/DD4hep/src/DD4hepVolumeBuilder.cpp b/Plugins/DD4hep/src/DD4hepVolumeBuilder.cpp index 4b426d4c2b4..0913c3d31e1 100644 --- a/Plugins/DD4hep/src/DD4hepVolumeBuilder.cpp +++ b/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 diff --git a/Plugins/Detray/src/DetrayConverter.cpp b/Plugins/Detray/src/DetrayConverter.cpp index b4f19fcbaea..4f2ac983ad0 100644 --- a/Plugins/Detray/src/DetrayConverter.cpp +++ b/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::surface_payload surfacePayload; surfacePayload.transform = convertTransform(surface.transform(gctx)); @@ -176,13 +173,13 @@ std::vector Acts::DetrayConverter::convertPortal( std::array clipRange = {0., 0.}; std::vector 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 { diff --git a/Plugins/Podio/CMakeLists.txt b/Plugins/Podio/CMakeLists.txt index 18630685929..39cca792df3 100644 --- a/Plugins/Podio/CMakeLists.txt +++ b/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 diff --git a/Plugins/Podio/include/Acts/Plugins/Podio/PodioTrackContainer.hpp b/Plugins/Podio/include/Acts/Plugins/Podio/PodioTrackContainer.hpp index 0079c606bc5..8ef787b32c4 100644 --- a/Plugins/Podio/include/Acts/Plugins/Podio/PodioTrackContainer.hpp +++ b/Plugins/Podio/include/Acts/Plugins/Podio/PodioTrackContainer.hpp @@ -15,10 +15,14 @@ #include "Acts/EventData/detail/DynamicColumn.hpp" #include "Acts/Plugins/Podio/PodioDynamicColumns.hpp" #include "Acts/Plugins/Podio/PodioUtil.hpp" + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wold-style-cast" #include "ActsPodioEdm/ParticleHypothesis.h" #include "ActsPodioEdm/Track.h" #include "ActsPodioEdm/TrackCollection.h" #include "ActsPodioEdm/TrackInfo.h" +#pragma GCC diagnostic pop #include #include diff --git a/Plugins/Podio/include/Acts/Plugins/Podio/PodioTrackStateContainer.hpp b/Plugins/Podio/include/Acts/Plugins/Podio/PodioTrackStateContainer.hpp index d0331176edc..0a45c73d3cd 100644 --- a/Plugins/Podio/include/Acts/Plugins/Podio/PodioTrackStateContainer.hpp +++ b/Plugins/Podio/include/Acts/Plugins/Podio/PodioTrackStateContainer.hpp @@ -19,10 +19,14 @@ #include "Acts/Plugins/Podio/PodioUtil.hpp" #include "Acts/Utilities/HashedString.hpp" #include "Acts/Utilities/Helpers.hpp" + +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wold-style-cast" #include "ActsPodioEdm/BoundParametersCollection.h" #include "ActsPodioEdm/JacobianCollection.h" #include "ActsPodioEdm/TrackStateCollection.h" #include "ActsPodioEdm/TrackStateInfo.h" +#pragma GCC diagnostic pop #include #include @@ -33,8 +37,6 @@ #include #include -#include "podio/UserDataCollection.h" - namespace Acts { class MutablePodioTrackStateContainer; diff --git a/Plugins/Podio/src/PodioUtil.cpp b/Plugins/Podio/src/PodioUtil.cpp index 8e16e6ae3f4..2fd73c0db3a 100644 --- a/Plugins/Podio/src/PodioUtil.cpp +++ b/Plugins/Podio/src/PodioUtil.cpp @@ -217,9 +217,6 @@ void recoverDynamicColumns( std::unordered_map>& dynamic) { - using load_type = std::unique_ptr (*)( - const podio::CollectionBase*); - // See // https://github.com/AIDASoft/podio/blob/858c0ff0b841705d1b18aafd57569fcbd1beda91/include/podio/UserDataCollection.h#L30-L31 using types = TypeList(state.node->GetVolume()->GetShape()); // It uses the bounding box of TGeoBBox - // @TODO this should be replace by a proper TGeo to Acts::VolumeBounds - // and vertices converision which would make a more appropriate parsomg - double dx = options.unit * shape->GetDX(); - double dy = options.unit * shape->GetDY(); - double dz = options.unit * shape->GetDZ(); - for (auto x : std::vector{-dx, dx}) { - for (auto y : std::vector{-dy, dy}) { - for (auto z : std::vector{-dz, dz}) { - Vector3 edge = etrf * Vector3(x, y, z); - for (auto& check : options.parseRanges) { - double val = VectorHelpers::cast(edge, check.first); - if (val < check.second.first || val > check.second.second) { - accept = false; - break; + if (auto* shape = + dynamic_cast(state.node->GetVolume()->GetShape()); + shape != nullptr) { + // @TODO this should be replace by a proper TGeo to Acts::VolumeBounds + // and vertices converision which would make a more appropriate + // parsomg + double dx = options.unit * shape->GetDX(); + double dy = options.unit * shape->GetDY(); + double dz = options.unit * shape->GetDZ(); + for (auto x : std::vector{-dx, dx}) { + for (auto y : std::vector{-dy, dy}) { + for (auto z : std::vector{-dz, dz}) { + Vector3 edge = etrf * Vector3(x, y, z); + for (auto& check : options.parseRanges) { + double val = VectorHelpers::cast(edge, check.first); + if (val < check.second.first || val > check.second.second) { + accept = false; + break; + } } } } diff --git a/Tests/UnitTests/Core/Geometry/CylinderVolumeStackTests.cpp b/Tests/UnitTests/Core/Geometry/CylinderVolumeStackTests.cpp index 93141684032..fcdd21b931c 100644 --- a/Tests/UnitTests/Core/Geometry/CylinderVolumeStackTests.cpp +++ b/Tests/UnitTests/Core/Geometry/CylinderVolumeStackTests.cpp @@ -868,7 +868,7 @@ BOOST_DATA_TEST_CASE(Baseline, std::transform( volumes.begin(), volumes.end(), std::back_inserter(originalBounds), [](const auto& vol) { - return *dynamic_cast(&vol->volumeBounds()); + return dynamic_cast(vol->volumeBounds()); }); if (f < 0.0) { diff --git a/Tests/UnitTests/Core/Material/GridSurfaceMaterialTests.cpp b/Tests/UnitTests/Core/Material/GridSurfaceMaterialTests.cpp index 999f89ea85b..910f6c9960f 100644 --- a/Tests/UnitTests/Core/Material/GridSurfaceMaterialTests.cpp +++ b/Tests/UnitTests/Core/Material/GridSurfaceMaterialTests.cpp @@ -62,7 +62,6 @@ BOOST_AUTO_TEST_CASE(GridIndexedMaterial_invalid_bound2Grid_Unconnected) { using EqBound = Acts::GridAxisGenerators::EqBound; using EqGrid = EqBound::grid_type; - using Point = EqGrid::point_t; EqBound eqBound{{0., 5.}, 5}; EqGrid eqGrid{eqBound()}; @@ -86,7 +85,6 @@ BOOST_AUTO_TEST_CASE(GridIndexedMaterial_invalid_global2Grid_Unconnected) { using EqBound = Acts::GridAxisGenerators::EqBound; using EqGrid = EqBound::grid_type; - using Point = EqGrid::point_t; EqBound eqBound{{0., 5.}, 5}; EqGrid eqGrid{eqBound()}; diff --git a/Tests/UnitTests/Core/Material/MaterialInteractionAssignmentTests.cpp b/Tests/UnitTests/Core/Material/MaterialInteractionAssignmentTests.cpp index 1cf35a058af..5d6101cd3be 100644 --- a/Tests/UnitTests/Core/Material/MaterialInteractionAssignmentTests.cpp +++ b/Tests/UnitTests/Core/Material/MaterialInteractionAssignmentTests.cpp @@ -277,8 +277,6 @@ BOOST_AUTO_TEST_CASE(AssignWithPathLength) { Surface::makeShared(Transform3::Identity(), 20.0, 100.0); surface->assignGeometryId(GeometryIdentifier().setSensitive(1)); - using SurfaceHit = std::tuple; - Vector3 position = {20., 10., 0.}; Vector3 direction = position.normalized(); diff --git a/Tests/UnitTests/Core/Propagator/MultiStepperTests.cpp b/Tests/UnitTests/Core/Propagator/MultiStepperTests.cpp index be4b0d81661..dea64db2a50 100644 --- a/Tests/UnitTests/Core/Propagator/MultiStepperTests.cpp +++ b/Tests/UnitTests/Core/Propagator/MultiStepperTests.cpp @@ -751,9 +751,11 @@ void propagator_instatiation_test_function() { decltype(propagator.template propagate( pars, *surface, options)); + static_assert(!std::is_same_v); // Instantiate without target - using tybe_b = decltype(propagator.propagate(pars, options)); + using type_b = decltype(propagator.propagate(pars, options)); + static_assert(!std::is_same_v); } BOOST_AUTO_TEST_CASE(propagator_instatiation_test) { diff --git a/Tests/UnitTests/Core/Vertexing/ZScanVertexFinderTests.cpp b/Tests/UnitTests/Core/Vertexing/ZScanVertexFinderTests.cpp index 5b4ba624346..411ddda7cf2 100644 --- a/Tests/UnitTests/Core/Vertexing/ZScanVertexFinderTests.cpp +++ b/Tests/UnitTests/Core/Vertexing/ZScanVertexFinderTests.cpp @@ -101,8 +101,6 @@ BOOST_AUTO_TEST_CASE(zscan_finder_test) { // Set up propagator with void navigator auto propagator = std::make_shared(stepper); - using BilloirFitter = FullBilloirVertexFitter; - // Create perigee surface std::shared_ptr perigeeSurface = Surface::makeShared(Vector3(0., 0., 0.)); @@ -220,8 +218,6 @@ BOOST_AUTO_TEST_CASE(zscan_finder_usertrack_test) { // Set up propagator with void navigator auto propagator = std::make_shared(stepper); - using BilloirFitter = FullBilloirVertexFitter; - // Create perigee surface std::shared_ptr perigeeSurface = Surface::makeShared(Vector3(0., 0., 0.)); diff --git a/Tests/UnitTests/Plugins/EDM4hep/ConvertTrackEDM4hepTest.cpp b/Tests/UnitTests/Plugins/EDM4hep/ConvertTrackEDM4hepTest.cpp index d3636eef0e2..ffd2ef2e7d4 100644 --- a/Tests/UnitTests/Plugins/EDM4hep/ConvertTrackEDM4hepTest.cpp +++ b/Tests/UnitTests/Plugins/EDM4hep/ConvertTrackEDM4hepTest.cpp @@ -263,7 +263,6 @@ BOOST_AUTO_TEST_CASE(RoundTripTests) { auto trackStateContainer = std::make_shared(); TrackContainer tracks(trackContainer, trackStateContainer); - using mutable_proxy_t = decltype(tracks)::TrackProxy; using const_proxy_t = decltype(tracks)::ConstTrackProxy; std::mt19937 rng{42}; diff --git a/cmake/ActsCompilerOptions.cmake b/cmake/ActsCompilerOptions.cmake index 5646f972a65..64467c64665 100644 --- a/cmake/ActsCompilerOptions.cmake +++ b/cmake/ActsCompilerOptions.cmake @@ -9,7 +9,9 @@ if(NOT CMAKE_BUILD_TYPE) message(STATUS "Setting default build type: ${CMAKE_BUILD_TYPE}") endif() -set(cxx_flags "-Wall -Wextra -Wpedantic -Wshadow -Wno-unused-local-typedefs") +set(cxx_flags + "-Wall -Wextra -Wpedantic -Wshadow -Wzero-as-null-pointer-constant -Wold-style-cast" +) # This adds some useful conversion checks like float-to-bool, float-to-int, etc. # However, at the moment this is only added to clang builds, since GCC's -Wfloat-conversion @@ -18,6 +20,18 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") set(cxx_flags "${cxx_flags} -Wfloat-conversion") endif() +# -Wnull-dereference gets applied to -isystem includes in GCC13, +# which causes lots of warnings in code we have no control over +if( + CMAKE_CXX_COMPILER_ID MATCHES "Clang" + OR ( + CMAKE_CXX_COMPILER_ID STREQUAL "GNU" + AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS_EQUAL 12 + ) +) + set(cxx_flags "${cxx_flags} -Wnull-dereference") +endif() + set(ACTS_CXX_STANDARD 20) set(ACTS_CXX_STANDARD_FEATURE cxx_std_20) if(DEFINED CMAKE_CXX_STANDARD) diff --git a/thirdparty/OpenDataDetector b/thirdparty/OpenDataDetector index 5d9950adb3e..0bf11ff33bd 160000 --- a/thirdparty/OpenDataDetector +++ b/thirdparty/OpenDataDetector @@ -1 +1 @@ -Subproject commit 5d9950adb3e264e717daef92500eaa304f370b6d +Subproject commit 0bf11ff33bd10d3070e6ecbc4bea0a7c42d86fa0 diff --git a/thirdparty/traccc/CMakeLists.txt b/thirdparty/traccc/CMakeLists.txt index 0cbf13838c9..6271b5ef0c3 100644 --- a/thirdparty/traccc/CMakeLists.txt +++ b/thirdparty/traccc/CMakeLists.txt @@ -15,7 +15,12 @@ message(STATUS "Building traccc as part of the Acts project") set(TRACCC_VERSION "${_acts_traccc_version}") # Declare where to get traccc from. -FetchContent_Declare(traccc ${ACTS_TRACCC_SOURCE}) +FetchContent_Declare( + traccc + ${ACTS_TRACCC_SOURCE} + # Needed until https://github.com/acts-project/traccc/pull/682 and https://github.com/acts-project/traccc/pull/683 are deployed + PATCH_COMMAND patch -p1 -i ${CMAKE_CURRENT_SOURCE_DIR}/warnings.diff +) if(ACTS_CUSTOM_SCALARTYPE) set(ACTS_TRACCC_SCALARTYPE ${ACTS_CUSTOM_SCALARTYPE}) @@ -46,6 +51,11 @@ set(TRACCC_SETUP_DFELIBS set(TRACCC_SETUP_DETRAY ON CACHE BOOL "Set up Detray as part of Traccc") set(TRACCC_SETUP_ACTS OFF CACHE BOOL "Do not set up ACTS as part of Traccc") set(TRACCC_SETUP_TBB OFF CACHE BOOL "Do not set up TBB as part of Traccc") +set(TRACCC_SETUP_BENCHMARKS + OFF + CACHE BOOL + "Do not set up google benchmarks as part of Traccc" +) set(TRACCC_BUILD_TESTING OFF diff --git a/thirdparty/traccc/warnings.diff b/thirdparty/traccc/warnings.diff new file mode 100644 index 00000000000..78fa1f37118 --- /dev/null +++ b/thirdparty/traccc/warnings.diff @@ -0,0 +1,40 @@ +diff --git a/core/include/traccc/seeding/spacepoint_binning_helper.hpp b/core/include/traccc/seeding/spacepoint_binning_helper.hpp +index 56a068d4..5a4f2ff2 100644 +--- a/core/include/traccc/seeding/spacepoint_binning_helper.hpp ++++ b/core/include/traccc/seeding/spacepoint_binning_helper.hpp +@@ -99,7 +99,8 @@ inline std::pair, detray::axis2::regular<>> get_axes( + + scalar zBinSize = grid_config.cotThetaMax * grid_config.deltaRMax; + detray::dindex zBins = std::max( +- 1, (int)std::floor((grid_config.zMax - grid_config.zMin) / zBinSize)); ++ 1, static_cast( ++ std::floor((grid_config.zMax - grid_config.zMin) / zBinSize))); + + detray::axis2::regular m_z_axis{zBins, grid_config.zMin, grid_config.zMax, + mr}; +diff --git a/io/src/csv/read_cells.cpp b/io/src/csv/read_cells.cpp +index a80d8f92..1451202f 100644 +--- a/io/src/csv/read_cells.cpp ++++ b/io/src/csv/read_cells.cpp +@@ -58,7 +58,7 @@ traccc::cell_module get_module(const std::uint64_t geometry_id, + } + + // Set the value on the module description. +- result.placement = (*geom)[result.surface_link.value()]; ++ result.placement = geom->at(result.surface_link.value()); + } + + // Find/set the digitization configuration of the detector module. +diff --git a/io/src/csv/read_spacepoints.cpp b/io/src/csv/read_spacepoints.cpp +index b2a38aba..41922683 100644 +--- a/io/src/csv/read_spacepoints.cpp ++++ b/io/src/csv/read_spacepoints.cpp +@@ -59,7 +59,7 @@ void read_spacepoints(spacepoint_reader_output& out, std::string_view filename, + m[iohit.geometry_id] = link; + cell_module mod; + mod.surface_link = detray::geometry::barcode{iohit.geometry_id}; +- mod.placement = geom[iohit.geometry_id]; ++ mod.placement = geom.at(iohit.geometry_id); + result_modules.push_back(mod); + } +