From 193b6ba3e8c3dabdba7bef9b95b4b135d8243e50 Mon Sep 17 00:00:00 2001 From: Roland Reichwein Date: Wed, 10 Sep 2025 11:41:09 +0200 Subject: [PATCH] Fix etl::typed_storage by supporting omitted destructors (#1182) * Added coderabbitai configuration * Added builtin mem function tests * Modified etl::typed_storage * Modified etl::typed_storage # Conflicts: # include/etl/alignment.h * Added ETL_NOEXCEPT and ETL_NOEXCEPT_IF_NO_THROW * Added etl::typed_storage_ext and swap for same * Added etl::typed_storage_ext and swap for same # Conflicts: # include/etl/alignment.h * Added release notes * Fixes to GCC -O2 errors * Changed char* parameters to value_type* parameters * Fixed compilation issues for const containers unit tests * Added automatic selection of __builtin_memxxx functions for GCC and clang * Added enhanced coderabbit configuration * Updated version and release notes * Disabled constexpr const container tests for C++11 * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Attempted fixes for MacOS compilation * Updated version and release notes * feat: removed unreachable break statements (#1169) * Updated version and release notes * Modified etl::typed_storage # Conflicts: # include/etl/alignment.h * Fix etl::typed_storage by supporting omitted destructors In a recent change to alignment.h, the etl::typed_storage was changed in a way that in case of an already constructed object, the object is created via assignment. However, this contradicts the original use case that led to etl::typed_storage in the first place: https://github.com/ETLCPP/etl/pull/1023 The goal is to omit destructors (and at the same time support classes with deleted assignment operators), so they can be optimized out at link time. This change reverts commit ac7b268 to restore the original functionality and changes the test to reflect the original use case. * Fix missing create() in non-C++11 typed_storage_ext constructor * Typo fix --------- Co-authored-by: John Wellbelove Co-authored-by: Drew Rife Co-authored-by: John Wellbelove --- include/etl/alignment.h | 101 ++++++++++++++-------------------------- test/test_alignment.cpp | 12 ++++- 2 files changed, 47 insertions(+), 66 deletions(-) diff --git a/include/etl/alignment.h b/include/etl/alignment.h index c289b4a4..73527765 100644 --- a/include/etl/alignment.h +++ b/include/etl/alignment.h @@ -450,17 +450,10 @@ namespace etl template reference create(TArgs&&... args) ETL_NOEXCEPT_EXPR(ETL_NOT_USING_EXCEPTIONS) { - if (has_value()) - { - storage.value = T(args...); - } - else - { - valid = true; - ::new (&storage.value) value_type(etl::forward(args)...); - } - - return storage.value; + ETL_ASSERT(!has_value(), ETL_ERROR(etl::typed_storage_error)); + pointer p = ::new (&storage.value) value_type(etl::forward(args)...); + valid = true; + return *p; } #else //*************************************************************************** @@ -470,17 +463,10 @@ namespace etl template reference create(const T1& t1) { - if (has_value()) - { - storage.value = T(t1); - } - else - { - valid = true; - ::new (&storage.value) value_type(t1); - } - - return storage.value; + ETL_ASSERT(!has_value(), ETL_ERROR(etl::typed_storage_error)); + pointer p = ::new (&storage.value) value_type(t1); + valid = true; + return *p; } //*************************************************************************** @@ -490,17 +476,10 @@ namespace etl template reference create(const T1& t1, const T2& t2) { - if (has_value()) - { - storage.value = T(t1, t2); - } - else - { - valid = true; - ::new (&storage.value) value_type(t1, t2); - } - - return storage.value; + ETL_ASSERT(!has_value(), ETL_ERROR(etl::typed_storage_error)); + pointer p = ::new (&storage.value) value_type(t1, t2); + valid = true; + return *p; } //*************************************************************************** @@ -510,17 +489,10 @@ namespace etl template reference create(const T1& t1, const T2& t2, const T3& t3) { - if (has_value()) - { - storage.value = T(t1, t2, t3); - } - else - { - valid = true; - ::new (&storage.value) value_type(t1, t2, t3); - } - - return storage.value; + ETL_ASSERT(!has_value(), ETL_ERROR(etl::typed_storage_error)); + pointer p = ::new (&storage.value) value_type(t1, t2, t3); + valid = true; + return *p; } //*************************************************************************** @@ -530,17 +502,10 @@ namespace etl template reference create(const T1& t1, const T2& t2, const T3& t3, const T4& t4) { - if (has_value()) - { - storage.value = T(t1, t2, t3, t4); - } - else - { - valid = true; - ::new (&storage.value) value_type(t1, t2, t3, t4); - } - - return storage.value; + ETL_ASSERT(!has_value(), ETL_ERROR(etl::typed_storage_error)); + pointer p = ::new (&storage.value) value_type(t1, t2, t3, t4); + valid = true; + return *p; } #endif @@ -684,6 +649,7 @@ namespace etl , valid(false) { ETL_ASSERT(etl::is_aligned(pbuffer_, etl::alignment_of::value), ETL_ERROR(etl::alignment_error)); + create(t1); } //*************************************************************************** @@ -744,62 +710,67 @@ namespace etl #if ETL_USING_CPP11 //*************************************************************************** /// Constructs the instance of T forwarding the given \p args to its constructor. - /// \returns the instance of T which has been constructed in the internal byte array. + /// \returns the instance of T which has been constructed in the external buffer. //*************************************************************************** template reference create(TArgs&&... args) ETL_NOEXCEPT_EXPR(ETL_NOT_USING_EXCEPTIONS) { ETL_ASSERT(!has_value(), ETL_ERROR(etl::typed_storage_error)); + pointer p = ::new (pbuffer) value_type(etl::forward(args)...); valid = true; - return *::new (pbuffer) value_type(etl::forward(args)...); + return *p; } #else //*************************************************************************** /// Constructs the instance of T with type T1 - /// \returns the instance of T which has been constructed in the internal byte array. + /// \returns the instance of T which has been constructed in the external buffer. //*************************************************************************** template reference create(const T1& t1) { ETL_ASSERT(!has_value(), ETL_ERROR(etl::typed_storage_error)); + pointer p = ::new (pbuffer) value_type(t1); valid = true; - return *::new (pbuffer) value_type(t1); + return *p; } //*************************************************************************** /// Constructs the instance of T with types T1, T2 - /// \returns the instance of T which has been constructed in the internal byte array. + /// \returns the instance of T which has been constructed in the external buffer. //*************************************************************************** template reference create(const T1& t1, const T2& t2) { ETL_ASSERT(!has_value(), ETL_ERROR(etl::typed_storage_error)); + pointer p = ::new (pbuffer) value_type(t1, t2); valid = true; - return *::new (pbuffer) value_type(t1, t2); + return *p; } //*************************************************************************** /// Constructs the instance of T with types T1, T2, T3 - /// \returns the instance of T which has been constructed in the internal byte array. + /// \returns the instance of T which has been constructed in the external buffer. //*************************************************************************** template reference create(const T1& t1, const T2& t2, const T3& t3) { ETL_ASSERT(!has_value(), ETL_ERROR(etl::typed_storage_error)); + pointer p = ::new (pbuffer) value_type(t1, t2, t3); valid = true; - return *::new (pbuffer) value_type(t1, t2, t3); + return *p; } //*************************************************************************** /// Constructs the instance of T with types T1, T2, T3, T4 - /// \returns the instance of T which has been constructed in the internal byte array. + /// \returns the instance of T which has been constructed in the external buffer. //*************************************************************************** template reference create(const T1& t1, const T2& t2, const T3& t3, const T4& t4) { ETL_ASSERT(!has_value(), ETL_ERROR(etl::typed_storage_error)); + pointer p = ::new (pbuffer) value_type(t1, t2, t3, t4); valid = true; - return *::new (pbuffer) value_type(t1, t2, t3, t4); + return *p; } #endif diff --git a/test/test_alignment.cpp b/test/test_alignment.cpp index c38acedb..24ff4c28 100644 --- a/test/test_alignment.cpp +++ b/test/test_alignment.cpp @@ -48,6 +48,7 @@ void f(int) { } +// Demonstrator class for etl::typed_storage tests struct A_t { A_t(uint32_t v_x, uint8_t v_y) @@ -56,13 +57,22 @@ struct A_t { } + // Just for test purpose. In production code, etl::typed_storage + // actually supports the use case of destructors being optimized + // away since they are not necessary for global objects that are + // never destroyed ~A_t() { x = 0; y = 0; } - bool operator==(A_t& other) + // etl::typed_storage helps implementing the use case of becoming + // independent of the destructor. By deleting the assignment operator, + // we make sure that the destructor is not linked + A_t& operator=(const A_t&) = delete; + + bool operator==(const A_t& other) const { return other.x == x && other.y == y; }