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:
#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 <[email protected]>
Co-authored-by: Drew Rife <[email protected]>
Co-authored-by: John Wellbelove <[email protected]>

Author: 4 people

include/etl/alignment.h

@@ -450,17 +450,10 @@ namespace etl
     template <typename... TArgs>
     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<TArgs>(args)...);
-      }
-
-      return storage.value;
+      ETL_ASSERT(!has_value(), ETL_ERROR(etl::typed_storage_error));
+      pointer p = ::new (&storage.value) value_type(etl::forward<TArgs>(args)...);
+      valid = true;
+      return *p;
     }
 #else
     //***************************************************************************
@@ -470,17 +463,10 @@ namespace etl
     template <typename T1>
     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 <typename T1, typename T2>
     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 <typename T1, typename T2, typename T3>
     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 <typename T1, typename T2, typename T3, typename T4>
     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<T>::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 <typename... TArgs>
     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<TArgs>(args)...);
       valid = true;
-      return *::new (pbuffer) value_type(etl::forward<TArgs>(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 <typename T1>
     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 <typename T1, typename T2>
     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 <typename T1, typename T2, typename T3>
     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 <typename T1, typename T2, typename T3, typename T4>
     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
 

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;
   }