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

Fix/do not store owning data as const #752

Merged
merged 6 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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);
mschimek marked this conversation as resolved.
Show resolved Hide resolved
// 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
Loading