Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require C++20 and update to C++20 #698

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
4bf306c
Require C++20 in podio and ROOT
jmcarcell Oct 18, 2024
39abb06
Use consteval when possible and remove checks for C++20
jmcarcell Oct 18, 2024
f891469
Add a few more constevals
jmcarcell Oct 18, 2024
41c482e
Use concepts and simplify templates
jmcarcell Oct 18, 2024
52b6239
Change enable_ifs by requires
jmcarcell Oct 18, 2024
9ad9f8a
Fix the documentation for links
jmcarcell Oct 18, 2024
6f4960e
Remove an unused header and use std::disjunction
jmcarcell Oct 18, 2024
3338d1e
Use algorithms from std::ranges
jmcarcell Oct 18, 2024
da49058
Use std::ranges in a couple more places
jmcarcell Nov 18, 2024
9249d1f
Use concepts when possible and add comments when it's not possible
jmcarcell Oct 19, 2024
795a269
Simplify in more places
jmcarcell Oct 20, 2024
10baaae
Remove dead code
jmcarcell Oct 20, 2024
a982af5
Use std::ranges::find
jmcarcell Oct 20, 2024
4f60846
Remove the ubuntu workflow since it is built with C++17
jmcarcell Oct 20, 2024
20bd01f
Update docs for the frame
jmcarcell Oct 20, 2024
cad2eb0
Fix missing endif
jmcarcell Oct 20, 2024
1ff6cfc
Add missing is_detected_v
jmcarcell Oct 20, 2024
5977132
Fix pre-commit
jmcarcell Oct 20, 2024
a3cabde
Upper-case the concept collectionType
jmcarcell Oct 22, 2024
805ec23
Add a minimum ROOT version with support for C++20
jmcarcell Oct 30, 2024
bf9a6de
Add back an ubuntu workflow with C++20
jmcarcell Oct 30, 2024
cb85c26
Update the ROOT version
jmcarcell Oct 30, 2024
5964a4a
Update README.md
jmcarcell Nov 18, 2024
00dd956
Change the format to C++20
jmcarcell Nov 18, 2024
aac10de
Make sure to also format links.md
jmcarcell Nov 18, 2024
c2618fe
Remove no longer applicable enable_if from doc
tmadlener Dec 18, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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()
jmcarcell marked this conversation as resolved.
Show resolved Hide resolved
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()
tmadlener marked this conversation as resolved.
Show resolved Hide resolved
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
Loading