Skip to content

[handle] guard self-assign on owning-handle assignments - #130

Open
krystophny wants to merge 1 commit into
TRIQS:unstablefrom
krystophny:fix/issue-11-self-move-assign-handle-heap
Open

[handle] guard self-assign on owning-handle assignments#130
krystophny wants to merge 1 commit into
TRIQS:unstablefrom
krystophny:fix/issue-11-self-move-assign-handle-heap

Conversation

@krystophny

Copy link
Copy Markdown

Fixes #11.

Add EXPECTS(this != &h) on the move-assign operators of the three
non-defaulted owning handles (handle_heap, handle_stack, handle_sso).
Also add if (this == &h) return *this; to the copy-assigns of
handle_heap and handle_stack (handle_sso already has it).
handle_borrowed is = default and unaffected. EXPECTS is a no-op under
NDEBUG, so release builds pay nothing on the hot path.

check_handle<H>() in test/c++/nda_mem.cpp deliberately exercised
std::swap(handle, handle) and move1 = std::move(move1), both now
precondition violations. Gated under #ifdef NDEBUG so release-mode
coverage is preserved.

Verification

$ cmake -S nda -B build/nda -G Ninja -DCMAKE_BUILD_TYPE=Debug -DBuild_Tests=ON
$ cmake --build build/nda -j && ctest --test-dir build/nda --output-on-failure -j
100% tests passed, 0 tests failed out of 119

$ cmake -S nda -B build/nda-release -G Ninja -DCMAKE_BUILD_TYPE=Release -DBuild_Tests=ON
$ cmake --build build/nda-release -j && ctest --test-dir build/nda-release --output-on-failure -j
100% tests passed, 0 tests failed out of 119

Move-assigns (handle_heap, handle_stack, handle_sso): add
EXPECTS(this != &h). EXPECTS is a no-op under NDEBUG, so release builds pay
nothing on the hot path.

Copy-assigns (handle_heap, handle_stack): add the standard fast-path
if (this == &h) return *this; (handle_sso already has it).

handle_borrowed is = default and unaffected.

check_handle<H>() in nda_mem.cpp deliberately exercised self-swap and
self-move-assign, both now precondition violations. Gated under
#ifdef NDEBUG so release-mode coverage is preserved.

Closes TRIQS#11.
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.

1 participant