fix(util): enforce nothrow move constraint on fixed_vector element type#1874
fix(util): enforce nothrow move constraint on fixed_vector element type#1874rishabhvaish wants to merge 3 commits intoEVerest:mainfrom
Conversation
rishabhvaish
requested review from
Pietfried,
SebaLukas,
andistorm,
cburandt,
corneliusclaussen,
djchhp,
florinmihut,
hikinggrass,
james-ctc and
mlitre
as code owners
|
Hello @rishabhvaish thank you for your contribution! The changes look good to me. However, can you make sure to format the changes correctly and fix the unit tests? It seems like some test are now irrelevant since we have removed support for throwable move / copy assignment/ constructions |
|
Thanks @mlitre for the review! I'll fix the formatting and remove/update the unit tests that are no longer relevant since the throwable move/copy support was removed. Will push an update shortly. |
|
Hey @rishabhvaish everything is looking pretty good! I still see some structs and tests that are no longer required (eg ThrowsOnCopy, etc). We would like to completely remove those and only have a test to show that objects that throw on copy / construction are no longer allowed correctly. |
Add static_assert to reject types with potentially throwing move constructors or move assignment operators. This eliminates the throwing-move code paths that silently swallowed exceptions, leaving callers with empty vectors and no error notification. The previous implementation provided two move paths: a fast noexcept path and a "strong guarantee" path for throwing moves. The latter caught exceptions internally and did not propagate them, which meant users had to defensively check for empty vectors after every move operation. By requiring nothrow move operations at compile time, we: - Prevent silent data loss from swallowed move exceptions - Remove ~140 lines of complex rollback/recovery logic - Make move construction and assignment unconditionally noexcept - Align with standard library containers' best practices for embedded/real-time use cases This constraint is compatible with all standard library types commonly used with fixed_vector (int, std::string, std::unique_ptr, etc.) as they all provide nothrow move operations. Closes EVerest#1814 Signed-off-by: Rishabh Vaish <rishabhvaish.904@gmail.com>
- Fix clang-format alignment issue in move_assign_from template parameter - Add explicit noexcept move constructors/assignment operators to ThrowsOnCopy and ThrowsOnNthCopy test types so they satisfy the new static_assert constraint while still testing copy-exception safety - Change ExceptionSafety test to pass const lvalue to push_back to ensure copy constructor (not move) is exercised Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Rishabh Vaish <rishabhvaish.904@gmail.com>
…d tests Remove ThrowsOnCopy, ThrowsOnDefaultConstruct, and ThrowsOnNthCopy structs along with their associated tests (ExceptionSafety, TryEmplaceBackExceptionDefaultConstruct, InitializerListStrongException). These are no longer relevant since fixed_vector now enforces nothrow move constraints via static_assert, rejecting types with throwing move operations at compile time. Add ThrowingMoveTypesAreRejected test that verifies via static_assert that types with throwing move constructors/assignments fail the trait checks that fixed_vector relies on for its compile-time rejection. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Rishabh Vaish <rishabhvaish.904@gmail.com>
rishabhvaish
force-pushed
the
fix/improve-fixed-vector-nothrow-constraint
branch
from
60640f7 to
f8d5115
Compare
|
Thanks @mlitre! Done in f8d5115 — removed Replaced them with a single |
|
@rishabhvaish looks good to me! You just to fix the linting issue and then we're good! Thank you for your contribution! |
2 participants
Summary
static_assertconstraints requiring element types with move operations to benoexcept, as requested in Improve fixed_vector util #1814noexceptstatic_assertverificationCloses #1814
Test plan
fixed_vectortests pass (removed tests for now-invalid throwing-move types)NothrowMoveConstrainttest verifiesstatic_assertworks forint,std::string, and custom nothrow-movable typesstatic_assertfires at compile time for types with throwing move constructors/assignmentSigned-off-by: Rishabh Vaish rishabhvaish.904@gmail.com
🤖 Generated with Claude Code