Skip to content

Commit

Permalink
Require C++20 and update to C++20 (#698)
Browse files Browse the repository at this point in the history
* Require C++20 in podio and ROOT

* Use consteval when possible and remove checks for C++20

* Use concepts and simplify templates

* Change enable_ifs by requires

* Fix the documentation for links

* Remove an unused header and use std::disjunction

* Use algorithms from std::ranges

* Use concepts when possible and add comments when it's not possible

* Remove dead code

* Use std::ranges::find

* Remove the ubuntu workflow since it is built with C++17

* Update docs for the frame

* Add missing is_detected_v

* Upper-case the concept collectionType

* Add a minimum ROOT version with support for C++20

* Add back an ubuntu workflow with C++20

* Update the ROOT version

* Update README.md

Co-authored-by: Thomas Madlener <[email protected]>

* Change the format to C++20

* Make sure to also format links.md

* Remove no longer applicable enable_if from doc

* [format] clang-format auto fixes

---------

Co-authored-by: jmcarcell <[email protected]>
Co-authored-by: Thomas Madlener <[email protected]>
  • Loading branch information
3 people authored Jan 8, 2025
1 parent a747cb0 commit 2e232bc
Show file tree
Hide file tree
Showing 27 changed files with 136 additions and 211 deletions.
2 changes: 1 addition & 1 deletion .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ SpacesInContainerLiterals: true
SpacesInCStyleCastParentheses: false
SpacesInParentheses: false
SpacesInSquareBrackets: false
Standard: c++17
Standard: c++20
TabWidth: 8
UseTab: Never
...
2 changes: 1 addition & 1 deletion .github/workflows/coverity.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
- uses: cvmfs-contrib/github-action-cvmfs@v4
- uses: aidasoft/run-lcg-view@v4
with:
coverity-cmake-command: 'cmake -DCMAKE_CXX_STANDARD=17 -DENABLE_SIO=ON -DENABLE_JULIA=ON -DUSE_EXTERNAL_CATCH2=OFF ..'
coverity-cmake-command: 'cmake -DCMAKE_CXX_STANDARD=20 -DENABLE_SIO=ON -DENABLE_JULIA=ON -DUSE_EXTERNAL_CATCH2=OFF ..'
coverity-project: 'AIDASoft%2Fpodio'
coverity-project-token: ${{ secrets.PODIO_COVERITY_TOKEN }}
github-pat: ${{ secrets.READ_COVERITY_IMAGE }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/edm4hep.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ jobs:
echo "::group::Build Catch2"
cd $STARTDIR/catch2
mkdir build && cd build
cmake -DCMAKE_CXX_STANDARD=17 -DCMAKE_INSTALL_PREFIX=../install -G Ninja ..
cmake -DCMAKE_CXX_STANDARD=20 -DCMAKE_INSTALL_PREFIX=../install -G Ninja ..
ninja -k0 install
export CMAKE_PREFIX_PATH=$STARTDIR/catch2/install:$CMAKE_PREFIX_PATH
echo "::endgroup::"
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/key4hep.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
cmake -DENABLE_SIO=ON \
-DENABLE_JULIA=OFF \
-DCMAKE_INSTALL_PREFIX=../install \
-DCMAKE_CXX_STANDARD=17 \
-DCMAKE_CXX_STANDARD=20 \
-DCMAKE_CXX_FLAGS=" -fdiagnostics-color=always -Werror -Wno-error=deprecated-declarations " \
-DUSE_EXTERNAL_CATCH2=AUTO \
-DENABLE_RNTUPLE=ON \
Expand Down
7 changes: 4 additions & 3 deletions .github/workflows/ubuntu.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

name: ubuntu

on:
Expand All @@ -13,8 +14,8 @@ jobs:
strategy:
fail-fast: false
matrix:
LCG: ["dev3/x86_64-ubuntu2204-gcc11-opt",
"dev4/x86_64-ubuntu2204-gcc11-opt"]
LCG: ["dev3/x86_64-ubuntu2404-gcc13-opt",
"dev4/x86_64-ubuntu2404-gcc13-opt"]
steps:
- uses: actions/checkout@v4
- uses: cvmfs-contrib/github-action-cvmfs@v4
Expand All @@ -30,7 +31,7 @@ jobs:
-DENABLE_JULIA=ON \
-DENABLE_DATASOURCE=ON \
-DCMAKE_INSTALL_PREFIX=../install \
-DCMAKE_CXX_STANDARD=17 \
-DCMAKE_CXX_STANDARD=20 \
-DCMAKE_CXX_FLAGS=" -fdiagnostics-color=always -Werror -Wno-error=deprecated-declarations " \
-DUSE_EXTERNAL_CATCH2=OFF \
-DPODIO_SET_RPATH=ON \
Expand Down
16 changes: 4 additions & 12 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ endif()
# ``-DCMAKE_CXX_STANDARD=<standard>`` when invoking CMake
set(CMAKE_CXX_STANDARD 20 CACHE STRING "")

if(NOT CMAKE_CXX_STANDARD MATCHES "17|20")
if(NOT CMAKE_CXX_STANDARD MATCHES "20|23")
message(FATAL_ERROR "Unsupported C++ standard: ${CMAKE_CXX_STANDARD}")
endif()

Expand Down Expand Up @@ -88,10 +88,7 @@ endif()
if(ENABLE_DATASOURCE)
list(APPEND root_components_needed ROOTDataFrame)
endif()
find_package(ROOT REQUIRED COMPONENTS ${root_components_needed})
if((ENABLE_RNTUPLE) AND (${ROOT_VERSION} VERSION_LESS 6.28.02))
message(FATAL_ERROR "You are trying to build podio with support for the new ROOT NTuple format, but your ROOT version is too old. Please update ROOT to at least version 6.28.02")
endif()
find_package(ROOT 6.28.04 REQUIRED COMPONENTS ${root_components_needed})

# ROOT_CXX_STANDARD was introduced in https://github.com/root-project/root/pull/6466
# before that it's an empty variable so we check if it's any number > 0
Expand All @@ -112,18 +109,13 @@ else()
message(STATUS "Determined ROOT c++ standard: " ${ROOT_CXX_STANDARD})
endif()

if(ROOT_CXX_STANDARD VERSION_LESS 17)
message(FATAL_ERROR "You are trying to build podio against a version of ROOT that has not been built with a sufficient c++ standard. podio requires c++17 or higher")
if(ROOT_CXX_STANDARD VERSION_LESS 20)
message(FATAL_ERROR "You are trying to build podio against a version of ROOT that has not been built with a sufficient c++ standard. podio requires c++20 or higher")
endif()
if(NOT ROOT_CXX_STANDARD VERSION_EQUAL CMAKE_CXX_STANDARD)
message(WARNING "You are trying to build podio with a different c++ standard than ROOT. C++${CMAKE_CXX_STANDARD} was required but ROOT was built with C++${ROOT_CXX_STANDARD}")
endif()

# ROOT only sets usage requirements from 6.14, so for
# earlier versions need to hack in INTERFACE_INCLUDE_DIRECTORIES
if(ROOT_VERSION VERSION_LESS 6.14)
set_property(TARGET ROOT::Core APPEND PROPERTY INTERFACE_INCLUDE_DIRECTORIES "${ROOT_INCLUDE_DIRS}")
endif()
list(APPEND PODIO_IO_HANDLERS ROOT)

# python setup (including python package discovery and install dir)
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ use a recent LCG or Key4hep stack release.

On Mac OS or Ubuntu, you need to install the following software.

### ROOT 6.08.06
### ROOT 6.28.04

Install ROOT 6.08.06 (or later) and set up your ROOT environment:
Install ROOT 6.28.04 (or later) built with c++20 support and set up your ROOT environment:

source <root_path>/bin/thisroot.sh

Expand Down
6 changes: 0 additions & 6 deletions cmake/podioConfig.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,6 @@ endif()
if(NOT TARGET podio::podio)
include("${CMAKE_CURRENT_LIST_DIR}/podioTargets.cmake")
include("${CMAKE_CURRENT_LIST_DIR}/podioMacros.cmake")

# ROOT only sets usage requirements from 6.14, so for
# earlier versions need to hack in INTERFACE_INCLUDE_DIRECTORIES
if(ROOT_VERSION VERSION_LESS 6.14)
set_property(TARGET podio::podio APPEND PROPERTY INTERFACE_INCLUDE_DIRECTORIES "${ROOT_INCLUDE_DIRS}")
endif()
endif()

check_required_components(podio)
Expand Down
4 changes: 2 additions & 2 deletions doc/frame.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ Some compilers and static code analysis tools are able to detect the accidental
For putting in parameters the basic principle is very similar, with the major difference being, that for *trivial* types `getParameter` will actually return by value.
For all use cases there is some `enable_if` machinery in place to ensure that only valid collections and valid parameter types can actually be used.
These checks also make sure that it is impossible to put in collections without handing over ownership to the `Frame`.
For all use cases there is a concept requirement to ensure that only valid collections and valid parameter types can actually be used.
Additional checks also make sure that it is impossible to put in collections without handing over ownership to the `Frame`.
### Usage examples for collection data
These are a few very basic usage examples that highlight the main functionality (and potential pitfalls).
Expand Down
24 changes: 12 additions & 12 deletions doc/links.md
Original file line number Diff line number Diff line change
Expand Up @@ -236,27 +236,27 @@ default handle types. This is ensured through `static_assert`s in the
`GetDefaultHandleType` helper templates are used to retrieve the correct type
from any `FromT` regardless of whether it is a mutable or a default handle type
With this in mind, effectively all mutating operations on `Link`s are
defined using [*SFINAE*](https://en.cppreference.com/w/cpp/language/sfinae)
using the following template structure (taking here `setFrom` as an example)
defined using the following template structure (taking here `setFrom` as an example)

```cpp
template <typename FromU,
typename = std::enable_if_t<Mutable &&
std::is_same_v<detail::GetDefaultHandleType<FromU>, FromT>>>
template <typename FromU>
requires(Mutable && std::is_same_v<detail::GetDefaultHandleType<FromU>, FromT> &&
detail::isDefaultHandleType<FromU>)
void setFrom(FromU value);
```

This is a SFINAE friendly way to ensure that this definition is only viable if
the following conditions are met
- The object this method is called on has to be `Mutable`. (first part inside the `std::enable_if`)
- The passed in `value` is either a `Mutable` or default class of type `FromT`. (second part inside the `std::enable_if`)
Compilation will fail unless the following conditions are met
- The object this method is called on has to be `Mutable`.
- The passed in `value` is either a `Mutable` or default class of type `FromT`.

In some cases the template signature looks like this

```cpp
template<bool Mut = Mutable,
typename = std::enable_if<Mut && Mutable>>
void setWeight(float weight);
template <bool Mut = Mutable>
requires(Mut && Mutable)
void setWeight(float value) {
m_obj->data.weight = value;
}
```

The reason to have a defaulted `bool` template parameter here is the same as the
Expand Down
35 changes: 14 additions & 21 deletions include/podio/Frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
#define PODIO_FRAME_H

#include "podio/CollectionBase.h"
#include "podio/CollectionBufferFactory.h"
#include "podio/CollectionIDTable.h"
#include "podio/FrameCategories.h" // mainly for convenience
#include "podio/GenericParameters.h"
Expand All @@ -23,17 +22,13 @@

namespace podio {

/// Alias template for enabling overloads only for Collections
/// Concept for enabling overloads only for Collection r-values
template <typename T>
using EnableIfCollection = typename std::enable_if_t<isCollection<T>>;
concept CollectionRValueType = CollectionType<T> && !std::is_lvalue_reference_v<T>;

/// Alias template for enabling overloads only for Collection r-values
/// Concept for enabling overloads for r-values
template <typename T>
using EnableIfCollectionRValue = typename std::enable_if_t<isCollection<T> && !std::is_lvalue_reference_v<T>>;

/// Alias template for enabling overloads for r-values
template <typename T>
using EnableIfRValue = typename std::enable_if_t<!std ::is_lvalue_reference_v<T>>;
concept RValueType = !std::is_lvalue_reference_v<T>;

namespace detail {
/// The minimal interface for raw data types
Expand Down Expand Up @@ -165,7 +160,7 @@ class Frame {
/// @tparam FrameDataT Arbitrary data container that provides access to the
/// collection buffers as well as the metadata, when
/// requested by the Frame.
template <typename FrameDataT, typename = EnableIfRValue<FrameDataT>>
template <RValueType FrameDataT>
Frame(FrameDataT&&);

/// A Frame is move-only
Expand Down Expand Up @@ -193,7 +188,7 @@ class Frame {
///
/// @returns A const reference to the collection if it is available or to
/// an empty (static) collection
template <typename CollT, typename = EnableIfCollection<CollT>>
template <CollectionType CollT>
const CollT& get(const std::string& name) const;

/// Get a collection pointer from the Frame by name.
Expand All @@ -218,7 +213,7 @@ class Frame {
///
/// @returns A const reference to the collection that has just been
/// inserted
template <typename CollT, typename = EnableIfCollectionRValue<CollT>>
template <CollectionRValueType CollT>
const CollT& put(CollT&& coll, const std::string& name);

/// (Destructively) move a collection into the Frame.
Expand All @@ -234,7 +229,7 @@ class Frame {
/// is supported by GenericParameters
/// @param key The name under which this parameter should be stored
/// @param value The value of the parameter. A copy will be put into the Frame
template <typename T, typename = podio::EnableIfValidGenericDataType<T>>
template <ValidGenericDataType T>
inline void putParameter(const std::string& key, T value) {
m_self->parameters().set(key, std::move(value));
}
Expand Down Expand Up @@ -271,7 +266,7 @@ class Frame {
/// is supported by GenericParameters
/// @param key The name under which this parameter should be stored
/// @param values The values of the parameter. A copy will be put into the Frame
template <typename T, typename = std::enable_if_t<detail::isInTuple<T, SupportedGenericDataTypes>>>
template <ValidGenericDataType T>
inline void putParameter(const std::string& key, std::initializer_list<T>&& values) {
putParameter<std::vector<T>>(key, std::move(values));
}
Expand All @@ -282,9 +277,8 @@ class Frame {
/// @param key The key under which the value is stored
///
/// @returns An optional holding the value if it is present
template <typename T>
template <ValidGenericDataType T>
inline auto getParameter(const std::string& key) const {
static_assert(podio::isSupportedGenericDataType<T>, "Unsupported parameter type");
return m_self->parameters().get<T>(key);
}

Expand All @@ -303,9 +297,8 @@ class Frame {
/// @tparam T The desired parameter type
///
/// @returns A vector of keys for this parameter type
template <typename T>
template <ValidGenericDataType T>
inline std::vector<std::string> getParameterKeys() const {
static_assert(podio::isSupportedGenericDataType<T>, "Unsupported parameter type");
return m_self->parameters().getKeys<T>();
}

Expand Down Expand Up @@ -374,11 +367,11 @@ template <typename FrameDataT>
Frame::Frame(std::unique_ptr<FrameDataT> data) : m_self(std::make_unique<FrameModel<FrameDataT>>(std::move(data))) {
}

template <typename FrameDataT, typename>
template <RValueType FrameDataT>
Frame::Frame(FrameDataT&& data) : Frame(std::make_unique<FrameDataT>(std::move(data))) {
}

template <typename CollT, typename>
template <CollectionType CollT>
const CollT& Frame::get(const std::string& name) const {
const auto* coll = dynamic_cast<const CollT*>(m_self->get(name));
if (coll) {
Expand All @@ -400,7 +393,7 @@ inline void Frame::put(std::unique_ptr<podio::CollectionBase> coll, const std::s
}
}

template <typename CollT, typename>
template <CollectionRValueType CollT>
const CollT& Frame::put(CollT&& coll, const std::string& name) {
const auto* retColl = static_cast<const CollT*>(m_self->put(std::make_unique<CollT>(std::move(coll)), name));
if (retColl) {
Expand Down
Loading

0 comments on commit 2e232bc

Please sign in to comment.