build: Take CMAKE_CXX_STANDARD into account (#1981)

We're currently setting cxx_std_17 target by target.
This PR changes this to 17 or higher, and it only sets it on ActsCore
and lets CMake propagate this to dependent targets.

Blocked by:
- #2002

.github/workflows/builds.yml

@@ -301,16 +301,13 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        image: 
-          - ubuntu2004
-          - ubuntu2204
-          - ubuntu2204_clang
-        std: [17, 20]
-        exclude:
-          - image: ubuntu2204
-            std: 17
+        include:
           - image: ubuntu2004
+            std: 17
+          - image: ubuntu2204_cpp20
             std: 20
+          - image: ubuntu2204_clang
+            std: 17
     container: ghcr.io/acts-project/${{ matrix.image }}:v41
     env:
       INSTALL_DIR: ${{ github.workspace }}/install

Alignment/CMakeLists.txt

@@ -1,10 +1,6 @@
 add_library(ActsAlignment SHARED
   src/Kernel/detail/AlignmentEngine.cpp)
 
-target_compile_features(
-  ActsAlignment
-  PUBLIC cxx_std_17)
-
 target_include_directories(
   ActsAlignment
   PUBLIC

CMakeLists.txt

@@ -146,15 +146,15 @@ set_option_if(ACTS_BUILD_PLUGIN_EXATRKX ACTS_BUILD_EXAMPLES_EXATRKX)
 # feature tests
 include(CheckCXXSourceCompiles)
 
-# function that tests if the root installation is compatible (i.e., compiled with C++17)
+# function that tests if the root installation is compatible
 function(check_root_compatibility)
   get_target_property(ROOT_INCLUDE_DIR ROOT::Core INTERFACE_INCLUDE_DIRECTORIES)
-  set(CMAKE_REQUIRED_FLAGS -std=c++17)
   set(CMAKE_REQUIRED_INCLUDES ${ROOT_INCLUDE_DIR})
-  check_cxx_source_compiles("#include <string>\n#include <TString.h>\nint main(){}" ROOT_COMPATIBILITY_CHECK)
-  if(NOT ROOT_COMPATIBILITY_CHECK)
-    message(FATAL_ERROR "Root installation is misconfigured. Ensure that your Root installation was compiled with C++17.")
-  endif()
+  #yolo
+  #check_cxx_source_compiles("#include <string>\n#include <TString.h>\nint main(){}" ROOT_COMPATIBILITY_CHECK)
+  #if(NOT ROOT_COMPATIBILITY_CHECK)
+  #  message(FATAL_ERROR "Root installation is misconfigured. Ensure that your Root installation was compiled.")
+  #endif()
 endfunction()
 
 # additional configuration and tools
@@ -358,6 +358,15 @@ if(ACTS_BUILD_EXAMPLES_HEPMC3)
 endif()
 if(ACTS_BUILD_EXAMPLES_PYTHIA8)
   find_package(Pythia8 REQUIRED)
+
+if(DEFINED CMAKE_CXX_STANDARD)
+  if(${CMAKE_CXX_STANDARD} GREATER_EQUAL 20)
+    message(WARNING "ACTS is configured to build with C++20."
+      "As of version 309, Pythia8 does not compile under C++20."
+      "You can find a patch file at: https://gist.github.com/paulgessinger/7a823b4015d4511cf829472291c8ddcf")
+  endif()
+endif()
+
 endif()
 # other dependencies
 if(ACTS_BUILD_DOCS)

Core/CMakeLists.txt

@@ -7,7 +7,7 @@ add_library(
   ActsCore SHARED "")
 target_compile_features(
   ActsCore
-  PUBLIC cxx_std_17)
+  PUBLIC ${ACTS_CXX_STANDARD_FEATURE})
 target_include_directories(
   ActsCore
   PUBLIC

Core/include/Acts/Seeding/BinnedSPGroup.hpp

@@ -85,7 +85,7 @@ class BinnedSPGroup {
   BinnedSPGroup() = delete;
 
   template <typename spacepoint_iterator_t, typename callable_t>
-  BinnedSPGroup<external_spacepoint_t>(
+  BinnedSPGroup(
       spacepoint_iterator_t spBegin, spacepoint_iterator_t spEnd,
       callable_t&& toGlobal,
       std::shared_ptr<const Acts::BinFinder<external_spacepoint_t>>

Core/include/Acts/SpacePointFormation/SpacePointBuilderOptions.hpp

@@ -8,6 +8,9 @@
 
 #pragma once
 
+#include "Acts/Definitions/Algebra.hpp"
+#include "Acts/Definitions/TrackParametrization.hpp"
+#include "Acts/Definitions/Units.hpp"
 #include "Acts/EventData/SourceLink.hpp"
 
 namespace Acts {
@@ -27,7 +30,6 @@ struct SpacePointBuilderOptions {
   double stripLengthTolerance = 0.01;
   /// Allowed increase of strip length wrt gaps between strips
   double stripLengthGapTolerance = 0.01;
-  SpacePointBuilderOptions() = default;
 };
 
 struct StripPairOptions {
@@ -43,7 +45,6 @@ struct StripPairOptions {
   double diffPhi2 = 1.;
   /// Accepted distance between two clusters
   double diffDist = 100. * UnitConstants::mm;
-  StripPairOptions() = default;
 };
 
 }  // namespace Acts

Core/include/Acts/TrackFitting/GsfOptions.hpp

@@ -84,7 +84,9 @@ struct GsfOptions {
 
   std::string_view finalMultiComponentStateColumn = "";
 
+#if __cplusplus < 202002L
   GsfOptions() = delete;
+#endif
 };
 
 }  // namespace Experimental

Core/include/Acts/Utilities/detail/grid_helper.hpp

@@ -97,18 +97,19 @@ class GlobalNeighborHoodIndices {
       return *this;
     }
 
-    bool operator==(const iterator& it) {
-      // We know when we've reached the end, so we don't need an end-iterator.
-      // Sadly, in C++, there has to be one. Therefore, we special-case it
-      // heavily so that it's super-efficient to create and compare to.
-      if (it.m_parent == nullptr) {
+    bool operator!=(const iterator& it) { return !(*this == it); }
+
+    bool isEqual(const iterator& b) const {
+      if (b.m_parent == nullptr) {
         return m_localIndicesIter[0] == m_parent->m_localIndices[0].end();
       } else {
-        return m_localIndicesIter == it.m_localIndicesIter;
+        return m_localIndicesIter == b.m_localIndicesIter;
       }
     }
 
-    bool operator!=(const iterator& it) { return !(*this == it); }
+    friend bool operator==(const iterator& a, const iterator& b) {
+      return a.isEqual(b);
+    }
 
    private:
     std::array<NeighborHoodIndices::iterator, DIM> m_localIndicesIter;

Core/src/EventData/CorrectedTransformationFreeToBound.cpp

@@ -83,9 +83,9 @@ Acts::detail::CorrectedFreeToBoundTransformer::operator()(
   covSqrt = U * D;
 
   // Define kappa = alpha*alpha*N
-  ActsScalar kappa = m_alpha * m_alpha * eFreeSize;
+  ActsScalar kappa = m_alpha * m_alpha * static_cast<double>(eFreeSize);
   // lambda = alpha*alpha*N - N
-  ActsScalar lambda = kappa - eFreeSize;
+  ActsScalar lambda = kappa - static_cast<double>(eFreeSize);
   // gamma = sqrt(labmda + N)
   ActsScalar gamma = std::sqrt(kappa);
 

Examples/Framework/CMakeLists.txt

@@ -31,10 +31,6 @@ acts_target_link_libraries_system(
 target_compile_definitions(
   ActsExamplesFramework
   PRIVATE BOOST_FILESYSTEM_NO_DEPRECATED)
-# set per-target c++17 requirement that will be propagated to linked targets
-target_compile_features(
-  ActsExamplesFramework
-  PUBLIC cxx_std_17)
 
 if(ACTS_USE_EXAMPLES_TBB)
   # newer DD4hep version require TBB and search interally for TBB in

Fatras/CMakeLists.txt

@@ -13,9 +13,6 @@ add_library(
   src/Physics/StandardInteractions.cpp
   src/Utilities/LandauDistribution.cpp
   src/Utilities/ParticleData.cpp)
-target_compile_features(
-  ActsFatras
-  PUBLIC cxx_std_17)
 target_include_directories(
   ActsFatras
   PUBLIC

Fatras/Geant4/CMakeLists.txt

@@ -9,9 +9,6 @@ target_compile_definitions(
 target_include_directories(
   ActsFatrasGeant4
   SYSTEM PUBLIC ${Geant4_INCLUDE_DIRS})
-target_compile_features(
-  ActsFatrasGeant4
-  PUBLIC cxx_std_17)
 target_include_directories(
   ActsFatrasGeant4
   PUBLIC

Plugins/DD4hep/include/Acts/Plugins/DD4hep/DD4hepLayerBuilder.hpp

@@ -16,11 +16,9 @@
 #include "Acts/Utilities/BinUtility.hpp"
 #include "Acts/Utilities/Logger.hpp"
 
-class TGeoMatrix;
+#include "DD4hep/DetElement.h"
 
-namespace dd4hep {
-class DetElement;
-}
+class TGeoMatrix;
 
 namespace Acts {
 

Plugins/DD4hep/include/Acts/Plugins/DD4hep/DD4hepVolumeBuilder.hpp

@@ -13,16 +13,14 @@
 #include "Acts/Surfaces/Surface.hpp"
 #include "Acts/Utilities/Logger.hpp"
 
+#include "DD4hep/DetElement.h"
+
 class TrackingVolume;
 using MutableTrackingVolumePtr = std::shared_ptr<TrackingVolume>;
 using MutableTrackingVolumeVector = std::vector<MutableTrackingVolumePtr>;
 
 class TGeoMatrix;
 
-namespace dd4hep {
-class DetElement;
-}
-
 namespace Acts {
 
 /// @brief build confined TrackingVolumes of one cylinder setup from DD4hep

Plugins/Legacy/include/Acts/Seeding/AtlasSeedFinder.hpp

@@ -50,7 +50,57 @@ class AtlasSeedFinder {
   ///////////////////////////////////////////////////////////////////
 
   AtlasSeedFinder();
-  virtual ~AtlasSeedFinder();
+  virtual ~AtlasSeedFinder() {
+    if (r_index != nullptr) {
+      delete[] r_index;
+    }
+    if (r_map != nullptr) {
+      delete[] r_map;
+    }
+    if (r_Sorted != nullptr) {
+      delete[] r_Sorted;
+    }
+
+    // Delete seeds
+    //
+    for (i_seed = l_seeds.begin(); i_seed != l_seeds.end(); ++i_seed) {
+      delete *i_seed;
+    }
+    // Delete space points for reconstruction
+    //
+    i_spforseed = l_spforseed.begin();
+    for (; i_spforseed != l_spforseed.end(); ++i_spforseed) {
+      delete *i_spforseed;
+    }
+    if (m_seedOutput != nullptr) {
+      delete m_seedOutput;
+    }
+
+    if (m_SP != nullptr) {
+      delete[] m_SP;
+    }
+    if (m_R != nullptr) {
+      delete[] m_R;
+    }
+    if (m_Tz != nullptr) {
+      delete[] m_Tz;
+    }
+    if (m_Er != nullptr) {
+      delete[] m_Er;
+    }
+    if (m_U != nullptr) {
+      delete[] m_U;
+    }
+    if (m_V != nullptr) {
+      delete[] m_V;
+    }
+    if (m_Zo != nullptr) {
+      delete[] m_Zo;
+    }
+    if (m_OneSeeds != nullptr) {
+      delete[] m_OneSeeds;
+    }
+  }
 
   ///////////////////////////////////////////////////////////////////
   // Methods to initialize tool for new event or region

Plugins/Legacy/include/Acts/Seeding/AtlasSeedFinder.ipp

@@ -88,62 +88,6 @@ Acts::Legacy::AtlasSeedFinder<SpacePoint>::AtlasSeedFinder() {
   m_CmSp.reserve(500);
 }
 
-///////////////////////////////////////////////////////////////////
-// Destructor
-///////////////////////////////////////////////////////////////////
-template <class SpacePoint>
-Acts::Legacy::AtlasSeedFinder<SpacePoint>::~AtlasSeedFinder<SpacePoint>() {
-  if (r_index != nullptr) {
-    delete[] r_index;
-  }
-  if (r_map != nullptr) {
-    delete[] r_map;
-  }
-  if (r_Sorted != nullptr) {
-    delete[] r_Sorted;
-  }
-
-  // Delete seeds
-  //
-  for (i_seed = l_seeds.begin(); i_seed != l_seeds.end(); ++i_seed) {
-    delete *i_seed;
-  }
-  // Delete space points for reconstruction
-  //
-  i_spforseed = l_spforseed.begin();
-  for (; i_spforseed != l_spforseed.end(); ++i_spforseed) {
-    delete *i_spforseed;
-  }
-  if (m_seedOutput != nullptr) {
-    delete m_seedOutput;
-  }
-
-  if (m_SP != nullptr) {
-    delete[] m_SP;
-  }
-  if (m_R != nullptr) {
-    delete[] m_R;
-  }
-  if (m_Tz != nullptr) {
-    delete[] m_Tz;
-  }
-  if (m_Er != nullptr) {
-    delete[] m_Er;
-  }
-  if (m_U != nullptr) {
-    delete[] m_U;
-  }
-  if (m_V != nullptr) {
-    delete[] m_V;
-  }
-  if (m_Zo != nullptr) {
-    delete[] m_Zo;
-  }
-  if (m_OneSeeds != nullptr) {
-    delete[] m_OneSeeds;
-  }
-}
-
 ///////////////////////////////////////////////////////////////////
 // Initialize tool for new event
 ///////////////////////////////////////////////////////////////////

Tests/CommonHelpers/CMakeLists.txt

@@ -11,9 +11,6 @@ target_compile_definitions(
   PRIVATE
     "ACTS_TEST_DATA_DIR=\"${acts_test_data_dir}\""
     BOOST_FILESYSTEM_NO_DEPRECATED)
-target_compile_features(
-  ActsTestsCommonHelpers
-  PUBLIC cxx_std_17)
 target_include_directories(
   ActsTestsCommonHelpers
   PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}>)

Tests/UnitTests/Core/EventData/MultiTrajectoryTests.cpp

@@ -1139,7 +1139,8 @@ BOOST_AUTO_TEST_CASE(MemoryStats) {
 
   for (int t = 0; t < type_axis.size(); t++) {
     for (int c = 0; c < column_axis.size(); c++) {
-      BOOST_CHECK_EQUAL(h.at(c, t), 0);
+      double v = h.at(c, t);
+      BOOST_CHECK_EQUAL(v, 0.0);
     }
   }
 
@@ -1154,10 +1155,11 @@ BOOST_AUTO_TEST_CASE(MemoryStats) {
     for (int c = 0; c < column_axis.size(); c++) {
       std::string key = column_axis.bin(c);
       BOOST_TEST_CONTEXT("column: " << key) {
+        double v = h.at(c, t);
         if (t == 0) {
-          BOOST_CHECK_NE((h.at(c, t)), 0);
+          BOOST_CHECK_NE(v, 0.0);
         } else {
-          BOOST_CHECK_EQUAL((h.at(c, t)), 0);
+          BOOST_CHECK_EQUAL(v, 0.0);
         }
       }
     }

cmake/ActsCompilerOptions.cmake

@@ -10,6 +10,15 @@ set(ACTS_CXX_FLAGS_MINSIZEREL "")
 set(ACTS_CXX_FLAGS_RELEASE "")
 set(ACTS_CXX_FLAGS_RELWITHDEBINFO "")
 
+set(ACTS_CXX_STANDARD_FEATURE cxx_std_17)
+if(DEFINED CMAKE_CXX_STANDARD)
+  if(${CMAKE_CXX_STANDARD} GREATER_EQUAL 17)
+    set(ACTS_CXX_STANDARD_FEATURE "cxx_std_${CMAKE_CXX_STANDARD}")
+  else()
+    message(ERROR "CMAKE_CXX_STANDARD=${CMAKE_CXX_STANDARD}, but ACTS requires C++ >=17")
+  endif()
+endif()
+
 # 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 
 # is much more aggressive and also triggers on e.g., double-to-float