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

extract() <=> data buffer is owning #610

Merged
merged 5 commits into from
Jan 8, 2024

Conversation

mschimek
Copy link
Member

@mschimek mschimek commented Jan 5, 2024

DataBuffer::extract() is now available if only if DataBuffer::is_owning, i.e. the data buffer owns its underlying storage.
The distinction between in/out buffers is now made in the result object and is (somewhat) decoupled from the existence of the extract() member function.

First step for enabling structured-binding ready result objects.

@mschimek mschimek requested a review from niklas-uhl January 5, 2024 13:49
@mschimek mschimek changed the title extract is now possible for owning data buffers extract() <=> data buffer is owning Jan 5, 2024
Copy link
Member

@niklas-uhl niklas-uhl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me (with minor suggestions). But I am really starting to be confused by in out owning and friends, we should really write some high-level documentation for this together with the whole structured binding stuff.

/// @brief Helper for implementing the extract_* functions in \ref MPIResult. Is \c true if the passed buffer type owns
/// its underlying storage and is an output buffer.
template <typename Buffer>
inline constexpr bool is_extractable = Buffer::is_owning& Buffer::is_out_buffer;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
inline constexpr bool is_extractable = Buffer::is_owning& Buffer::is_out_buffer;
inline constexpr bool is_extractable = Buffer::is_owning && Buffer::is_out_buffer;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this located in this header? I think this should be part of DataBuffer.

Nevermind, it makes sense with the specialization for ResultCategoryNotUsed. Still, I think we should have Buffer::is_extractable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be in the internal namespace?

@mschimek mschimek merged commit c89aa70 into main Jan 8, 2024
8 checks passed
@mschimek mschimek deleted the feature/make-extract-structured-binding-ready branch January 8, 2024 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants