Skip to content

Commit

Permalink
Fix/do not store owning data as const (#752)
Browse files Browse the repository at this point in the history
* In const owning data buffers: do not store the data as consts as this enforces copying the data once it is moved out

* fix formatting

* fix compile error

* remove accidentaly made edit

* fix test

* add check for vector<bool>
  • Loading branch information
mschimek authored Sep 30, 2024
1 parent cf21b01 commit a696927
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 34 deletions.
22 changes: 19 additions & 3 deletions include/kamping/data_buffer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,8 @@ template <
typename ValueType = default_value_type_tag>
class DataBuffer : private CopyMoveEnabler<enable_copy_construction_v<ownership>>, private Extractable {
public:
static_assert(!std::is_const_v<MemberType>, "Member Type should not be const qualified.");

static constexpr TParameterType parameter_type =
parameter_type_param; ///< The type of parameter this buffer represents.

Expand Down Expand Up @@ -411,6 +413,16 @@ class DataBuffer : private CopyMoveEnabler<enable_copy_construction_v<ownership>
MemberTypeWithConst,
MemberTypeWithConst&>; ///< The ContainerType as const or non-const (see ContainerTypeWithConst) and
///< reference or non-reference depending on ownership.
///
using StorageType = std::conditional_t<
is_owning,
MemberType,
MemberTypeWithConstAndRef>; ///< The type as which the underlying container will be stored. If the buffer is
///< owning, i.e. the underlying data is not referenced but stored directly, the
///< potential constness of the data is not reflected in StorageType as this would
///< enforce copying of the \c const data once it will be extracted. Modifying const
///< data is instead prevented by giving only const qualified access via
///< underlying() or data() in such case.

using value_type =
typename ValueTypeWrapper<!is_single_element, MemberType>::value_type; ///< Value type of the buffer.
Expand Down Expand Up @@ -586,21 +598,25 @@ class DataBuffer : private CopyMoveEnabler<enable_copy_construction_v<ownership>
///
/// @return Moves the underlying container out of the DataBuffer.
template <bool enabled = is_owning, std::enable_if_t<enabled, bool> = true>
MemberTypeWithConst extract() {
StorageType extract() {
static_assert(
ownership == BufferOwnership::owning,
"Moving out of a reference should not be done because it would leave "
"a users container in an unspecified state."
);
static_assert(
!is_vector_bool_v<MemberType>,
"Buffers based on std::vector<bool> are not supported, use std::vector<kamping::kabool> instead."
);
kassert_not_extracted("Cannot extract a buffer that has already been extracted.");
auto extracted = std::move(underlying());
auto extracted = std::move(_data);
// we set is_extracted here because otherwise the call to underlying() would fail
set_extracted();
return extracted;
}

private:
MemberTypeWithConstAndRef _data; ///< Container which holds the actual data.
StorageType _data; ///< Container which holds the actual data.
};

/// @brief A more generic version of a DataBuffer which stores an object of type \tparam MemberType with its associcated
Expand Down
Loading

0 comments on commit a696927

Please sign in to comment.