Skip to content

Commit

Permalink
feat: Add ConstRefHolder type, add support to TrackContainer (#2035)
Browse files Browse the repository at this point in the history
This allows construction of `TrackContainer` from const references.
  • Loading branch information
paulgessinger committed Apr 19, 2023
1 parent 3d37973 commit 00e64ea
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 3 deletions.
19 changes: 19 additions & 0 deletions Core/include/Acts/EventData/TrackContainer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,21 @@ class TrackContainer {
TrackContainer(track_container_t& container, traj_t& traj)
: m_container{&container}, m_traj{&traj} {}

/// Constructor from const references to a track container backend and to a
/// track state container backend
/// @note The track container will not assume ownership over the backends in this case.
/// You need to ensure suitable lifetime
/// @param container the track container backend
/// @param traj the track state container backend
template <
template <typename> class H = holder_t,
bool RO = (IsReadOnlyTrackContainer<track_container_t>::value &&
IsReadOnlyMultiTrajectory<traj_t>::value),
typename = std::enable_if_t<
detail::is_same_template<H, detail::ConstRefHolder>::value && RO>>
TrackContainer(const track_container_t& container, const traj_t& traj)
: m_container{&container}, m_traj{&traj} {}

/// Get a const track proxy for a track index
/// @param itrack the track index in the container
/// @return A const track proxy for the index
Expand Down Expand Up @@ -275,6 +290,10 @@ template <typename track_container_t, typename traj_t>
TrackContainer(track_container_t& container, traj_t& traj)
-> TrackContainer<track_container_t, traj_t, detail::RefHolder>;

template <typename track_container_t, typename traj_t>
TrackContainer(const track_container_t& container, const traj_t& traj)
-> TrackContainer<track_container_t, traj_t, detail::ConstRefHolder>;

template <typename track_container_t, typename traj_t>
TrackContainer(track_container_t&& container, traj_t&& traj)
-> TrackContainer<track_container_t, traj_t, detail::ValueHolder>;
Expand Down
25 changes: 22 additions & 3 deletions Core/include/Acts/Utilities/Holders.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,17 @@

#pragma once

#include <utility>

namespace Acts::detail {

/// Internal holder type for referencing a backend without ownership
template <typename T>
struct RefHolder {
T* ptr;

RefHolder(T* _ptr) : ptr{_ptr} {}
RefHolder(T& ref) : ptr{&ref} {}
explicit RefHolder(T* _ptr) : ptr{_ptr} {}
explicit RefHolder(T& ref) : ptr{&ref} {}

const T& operator*() const { return *ptr; }
T& operator*() { return *ptr; }
Expand All @@ -25,6 +27,20 @@ struct RefHolder {
T* operator->() { return ptr; }
};

/// Internal holder type for referencing a backend without ownership that is
/// const
template <typename T>
struct ConstRefHolder {
const T* ptr;

explicit ConstRefHolder(const T* _ptr) : ptr{_ptr} {}
explicit ConstRefHolder(const T& ref) : ptr{&ref} {}

const T& operator*() const { return *ptr; }

const T* operator->() const { return ptr; }
};

/// Internal holder type holding a backend container by value
template <typename T>
struct ValueHolder {
Expand All @@ -33,7 +49,10 @@ struct ValueHolder {
// Let's be clear with the user that we take the ownership
// Only require rvalues and avoid hidden copies
ValueHolder(T& _val) = delete;
ValueHolder(T&& _val) : val{std::move(_val)} {}
// @FIXME: Ideally we want this to be explicit, but cannot be explicit,
// because using an explicit constructor and a deduction guide leads to
// a SEGFAULT in GCC11 (an up?). Re-evaluate down the line
/* explicit */ ValueHolder(T&& _val) : val{std::move(_val)} {}

// Does it makes sense to allow copy operations?

Expand Down
50 changes: 50 additions & 0 deletions Tests/UnitTests/Core/EventData/TrackTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "Acts/EventData/VectorMultiTrajectory.hpp"
#include "Acts/EventData/VectorTrackContainer.hpp"
#include "Acts/Tests/CommonHelpers/TestTrackState.hpp"
#include "Acts/Utilities/Holders.hpp"

#include <iterator>

Expand Down Expand Up @@ -444,6 +445,55 @@ BOOST_AUTO_TEST_CASE(ConstCorrectness) {
}
}

BOOST_AUTO_TEST_CASE(BuildFromConstRef) {
VectorTrackContainer mutVtc;
VectorMultiTrajectory mutMtj;

TrackContainer mutTc{mutVtc, mutMtj};
static_assert(!mutTc.ReadOnly, "Unexpectedly read only");

auto t = mutTc.getTrack(mutTc.addTrack());
t.appendTrackState();
t.appendTrackState();
t.appendTrackState();
t = mutTc.getTrack(mutTc.addTrack());
t.appendTrackState();

BOOST_CHECK_EQUAL(mutTc.size(), 2);
BOOST_CHECK_EQUAL(mutMtj.size(), 4);

ConstVectorTrackContainer vtc{std::move(mutVtc)};
ConstVectorMultiTrajectory mtj{std::move(mutMtj)};

// moved from
BOOST_CHECK_EQUAL(mutTc.size(), 0);
BOOST_CHECK_EQUAL(mutMtj.size(), 0);

TrackContainer ctc{vtc, mtj};
static_assert(ctc.ReadOnly, "Unexpectedly not read only");

// Does not compile:
// ctc.addTrack();

BOOST_CHECK_EQUAL(ctc.size(), 2);
BOOST_CHECK_EQUAL(mtj.size(), 4);

const auto& cvtc = vtc;
const auto& cmtj = mtj;

TrackContainer crtc{cvtc, cmtj};

BOOST_CHECK_EQUAL(crtc.size(), 2);
BOOST_CHECK_EQUAL(cmtj.size(), 4);

// Does not compile: holder deduced to ConstRefHolder, but is not RO
// const auto& mrvtc = mutVtc;
// const auto& mrmtj = mutMtj;
// TrackContainer mrtc{mrvtc, mrmtj};
// static_assert(ctc.ReadOnly, "Unexpectedly not read only");
// mrtc.addTrack();
}

BOOST_AUTO_TEST_CASE_TEMPLATE(BuildReadOnly, factory_t, const_holder_types) {
factory_t factory;
auto& tc = factory.trackContainer();
Expand Down

0 comments on commit 00e64ea

Please sign in to comment.