Repository navigation
Add C++17 uninitialized_move polyfill - #214
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation satisfies the requested API, rollback semantics, facade selection, and comprehensive test coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Adds a C++14-compatible uninitialized_move polyfill with exception-safe rollback and a C++17 facade.
Changes:
- Implements value-category-preserving construction and rollback.
- Exposes the polyfill through
cetl::uninitialized_move. - Adds comprehensive native and embedded-profile tests.
| File | Description |
|---|---|
include/cetl/pf17/memory.hpp |
Implements the polyfill and reusable construction helper. |
include/cetl/pf17/cetlpf.hpp |
Adds the version-selected facade. |
cetlvast/suites/unittest/test_pf17_memory.cpp |
Tests semantics, failures, iterators, and lifetimes. |
cetlvast/suites/unittest/test_pf17_memory_exception.hpp |
Declares embedded exception injection. |
cetlvast/suites/unittest/test_pf17_memory_exception.cpp |
Implements exception injection. |
cetlvast/suites/unittest/CMakeLists.txt |
Registers tests and exception-enabled helper compilation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
pavel-kirienko
approved these changes
Oct 6, 2026
thirtytwobits
added a commit
that referenced
this pull request
Oct 6, 2026
When an element constructor threw during a VLA bulk operation, already-constructed destination objects were not destroyed. Failed constructors and replacement-buffer relocation could also leak allocations. This change rolls back the constructed prefix and releases abandoned storage while keeping surviving containers usable. Currently stacked on #214 for review; the base is `codex/issue-206-uninitialized-move`. The VLA implementation is self-contained and has no dependency on the polyfill header. A VLA-local construction loop in the base tracks completed elements and destroys that prefix on failure, retaining allocator-aware construction/destruction, VLA's `move_if_noexcept` policy, and trivial-type fast paths. The shared base owns storage cleanup, including when a derived constructor fails. Copy, move, default, and fill construction commit size only after success; replacement allocations are released if relocation fails. Assignment can retain already-assigned values or leave an empty destination after replacing its old buffer. Throwing move-only elements may leave source values changed, as documented. The regression suite injects first/middle/final failures into initializer-list/range/copy construction, unequal-allocator moves, reserve/shrink relocation, resize, and assignment. It checks exact object lifetimes, allocator hooks, allocation ownership and sizes, and container reuse after failure. It also covers failures from allocator construction hooks and source-range access, empty ranges, propagating allocators, throwing assignments, packed-bool range failures, and packed-bool bookkeeping after allocation failure. Incorrect unconditional `noexcept` on copy-assignment helpers is removed so those failures can propagate. Validation: - Reproduced constructor leaks with the new tests against the unchanged #214 implementation. - All nine VLA suites passed GCC/Clang × C++14/17 × Debug/ReleaseEP (8 configurations). The focused exception-safety suite built and ran across GCC/Clang × C++14/17/20/23 × Debug/Release/DebugEP/ReleaseEP (32 configurations; exception cases are skipped in embedded profiles). - Isolated builds with only `variable_length_array.hpp` and `cetl.hpp` available passed GCC/Clang C++14/17 with and without exceptions. - GCC C++14 coverage of the local construction helper: all 9 executable lines covered; 7/8 branches covered. The remaining compiler-generated branch represents an allocator destroy operation throwing, which violates the documented nonthrowing destruction requirement. - All 14 exception-safety tests passed ASan/UBSan with leak detection in C++14 and C++17. Broader VLA sanitizer runs completed with existing UBSan findings for empty bool assignment (`memset(nullptr, ..., 0)`) and null deallocation through `std::pmr::polymorphic_allocator`; both were independently reproduced against the unchanged #214 header. - Doxygen completed with no new warnings; the same 13 existing warnings in `type_traits_ext.hpp` and `unbounded_variant.hpp` remain. - Clang-format and `git diff --check` passed. Closes #37 after this stack is merged into the default branch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the sequential three-argument
cetl::pf17::uninitialized_movefor C++14 and exposes it throughcetl::uninitialized_move, which selects the standard library implementation in C++17 and newer.The polyfill constructs into caller-owned storage and destroys the successfully constructed destination prefix if construction or a source iterator operation throws. Source value categories are preserved according to LWG 3918, including guaranteed elision for prvalue sources when the polyfill is used explicitly in C++17 and newer. An internal callback-based helper supports allocator-aware lifetime operations.
Tests cover move-only and copy-only values, const and single-pass sources, forward destinations, empty ranges, overloaded address-of and placement new, first/middle/last construction failures, iterator and stream failures, and exact rollback lifetimes. Embedded-profile death tests inject exceptions from a separate exception-enabled translation unit and verify termination while the algorithm and calling tests remain compiled with
-fno-exceptions.Validation:
-fno-elide-constructors; Clang C++14/17 ASan/UBSan checks passed.type_traits_ext.hppandunbounded_variant.hpp.git diff --checkpassed.Closes #206.