Skip to content

Invert meaning of operator bool for CUDA stream and event#199

Open
msimberg wants to merge 1 commit intoghex-org:masterfrom
msimberg:operator-bool-invert
Open

Invert meaning of operator bool for CUDA stream and event#199
msimberg wants to merge 1 commit intoghex-org:masterfrom
msimberg:operator-bool-invert

Conversation

@msimberg
Copy link
Copy Markdown
Collaborator

This was a leftover from #190. See #190 (comment).

@msimberg msimberg marked this pull request as ready for review March 20, 2026 13:02
@msimberg
Copy link
Copy Markdown
Collaborator Author

I see also that there actually already sort of existed a cudaEvent_t wrapper... just hidden inside the future helper. I'll try to unify the event wrapper added in #190 with the existing one in a separate PR.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aligns the boolean conversion semantics of CUDA stream/event wrappers with typical C++ “valid object == true” expectations (and with the GHEX_C_STRUCT / GHEX_C_MANAGED_STRUCT behavior referenced in the linked #190 discussion), by making moved-from objects evaluate to false.

Changes:

  • Invert operator bool() for ghex::device::stream and ghex::device::cuda_event to return true when the wrapper is usable (not moved-from).
  • Update the event_pool internal assertion to match the new cuda_event boolean semantics.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
include/ghex/device/cuda/stream.hpp stream::operator bool() now reports validity (!m_moved).
include/ghex/device/cuda/event.hpp cuda_event::operator bool() now reports validity (!m_moved).
include/ghex/device/cuda/event_pool.hpp Adjust assertion in get_event() to reflect the new cuda_event truthiness.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@msimberg
Copy link
Copy Markdown
Collaborator Author

I see also that there actually already sort of existed a cudaEvent_t wrapper... just hidden inside the future helper. I'll try to unify the event wrapper added in #190 with the existing one in a separate PR.

I opened #200.

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