Fix unaligned access and simplify code

src/include/amc_vectorcommon.hpp

@@ -331,22 +331,22 @@ class ElemStorage {
 };
 
 /// This class represents a merge of a pointer and some inline storage elements.
-/// When it is possible, the space is shared between them.
 /// Thanks to this optimization, SmallVector behaves like a string type with SSO
 /// Example : for a system with pointer size of 8 bytes,
 ///           sizeof(SmallVector<char, 8>) == sizeof(vector<char>)
 ///           because 8 chars can be stored in a pointer.
-template <class T, bool MergeTAndPtrStorage>
-class ElemWithPtrStorageImpl {
+template <class T>
+class ElemWithPtrStorage {
  public:
   using pointer = T *;
   using const_pointer = const T *;
+
   static constexpr bool kExtraSlots =
-      sizeof(pointer) > sizeof(T) && std::alignment_of<T *>::value % std::alignment_of<T>::value == 0 &&
-      sizeof(pointer) % sizeof(T) == 0;
+      sizeof(T *) > sizeof(T) && std::alignment_of<T *>::value % std::alignment_of<T>::value == 0 &&
+      sizeof(T *) % sizeof(T) == 0;
 
 #ifdef AMC_CXX14
-  // this is to avoid potential harmless warning occurring for instance in GCC:
+  // Use std::divides instead of '/' to avoid potential harmless warning occurring for instance in GCC:
   // warning: division 'sizeof (...) / sizeof (...)' does not compute the number of array elements
   // [-Wsizeof-pointer-div]
   // Activated only in C++14 as we need it to be constexpr
@@ -360,9 +360,9 @@ class ElemWithPtrStorageImpl {
   pointer ptr() noexcept { return reinterpret_cast<pointer>(this); }
   const_pointer ptr() const noexcept { return reinterpret_cast<const_pointer>(this); }
 
-  // Return the pointer stored in the first bytes of this object to the dynamic storage.
   void setDyn(pointer p) noexcept { std::memcpy(std::addressof(_el), std::addressof(p), sizeof(pointer)); }
 
+  // Return the pointer stored in the first bytes of this object to the dynamic storage.
   pointer dyn() const noexcept {
     // use memcpy to avoid breaking strict aliasing rule, will be optimized away by the compiler.
     // (confirmed with clang and gcc from O2)
@@ -383,34 +383,6 @@ class ElemWithPtrStorageImpl {
 #endif
 };
 
-template <class T>
-class ElemWithPtrStorageImpl<T, false> {
- public:
-  using pointer = T *;
-  using const_pointer = const T *;
-
-  static constexpr uint8_t kNbSlots = 1;
-
-  // Get a pointer to its underlying storage
-  pointer ptr() noexcept { return reinterpret_cast<pointer>(std::addressof(_el)); }
-  const_pointer ptr() const noexcept { return reinterpret_cast<const_pointer>(std::addressof(_el)); }
-
-  // Return the pointer stored in the first bytes of this object to the dynamic storage.
-  void setDyn(pointer p) noexcept { std::memcpy(std::addressof(_dyn), std::addressof(p), sizeof(pointer)); }
-  pointer dyn() const noexcept { return *reinterpret_cast<T *const *const>(std::addressof(_dyn)); }
-
- private:
-  // Ensure pointer is aligned with T such that alignment of ElemWithPtrStorageImpl is the same as T
-  typename std::aligned_storage<sizeof(pointer), std::alignment_of<T>::value>::type _dyn;
-  typename std::aligned_storage<sizeof(T), std::alignment_of<T>::value>::type _el;
-};
-
-template <class T>
-struct ElemWithPtrStorage
-    : public ElemWithPtrStorageImpl<T, sizeof(T) >= sizeof(T *) ||
-                                           (std::alignment_of<T *>::value % std::alignment_of<T>::value == 0 &&
-                                            sizeof(T *) % sizeof(T) == 0)> {};
-
 template <class T>
 void SwapDynStorage(ElemWithPtrStorage<T> &lhs, ElemWithPtrStorage<T> &rhs) {
   T *pTemp = lhs.dyn();

src/test/testtypes.hpp

@@ -273,28 +273,35 @@ struct MoveForbidden {
       typename std::conditional<IsTriviallyRelocatable, std::true_type, std::false_type>::type;
 };
 
-struct UnalignedToPtr1 {
-  UnalignedToPtr1(uint32_t i) { std::memcpy(c, std::addressof(i), sizeof(uint32_t)); }
+template <unsigned int Size>
+struct UnalignedToPtr {
+  static constexpr size_t kIntSize = Size < sizeof(uint32_t) ? Size : sizeof(uint32_t);
+
+  UnalignedToPtr(uint32_t i) { std::memcpy(c, &i, kIntSize); }
 
   operator uint32_t() const {
-    uint32_t ret;
-    std::memcpy(std::addressof(ret), c, sizeof(uint32_t));
+    uint32_t ret{};
+    std::memcpy(&ret, c, kIntSize);
     return ret;
   }
 
-  char c[5];
+  char c[Size];
 };
 
+template <unsigned int Size, class T>
 struct UnalignedToPtr2 {
-  UnalignedToPtr2(uint32_t i) { std::memcpy(c, std::addressof(i), 3); }
+  static constexpr size_t kIntSize = Size < sizeof(uint32_t) ? Size : sizeof(uint32_t);
+
+  UnalignedToPtr2(uint32_t i) { std::memcpy(c, &i, kIntSize); }
 
   operator uint32_t() const {
-    uint32_t ret = 0;
-    std::memcpy(std::addressof(ret), c, 3);
+    uint32_t ret{};
+    std::memcpy(&ret, c, kIntSize);
     return ret;
   }
 
-  char c[3];
+  T e;
+  char c[Size];
 };
 
 class TestAllocator {

src/test/vectors_test.cpp

@@ -32,7 +32,7 @@ typedef ::testing::Types<
     SmallVector<NonTriviallyRelocatableType, 1, std::allocator<NonTriviallyRelocatableType>, int16_t>,
     SmallVector<uint32_t, 0, std::allocator<uint32_t>, signed char>,
     SmallVector<NonTriviallyRelocatableType, 3, std::allocator<NonTriviallyRelocatableType>>,
-    SmallVector<UnalignedToPtr1, 4>, SmallVector<UnalignedToPtr2, 3>,
+    SmallVector<UnalignedToPtr<3>, 4>, SmallVector<UnalignedToPtr<7>, 3>, SmallVector<UnalignedToPtr<5>, 2>,
     vector<int32_t, std::allocator<int32_t>, uint64_t>, vector<TriviallyCopyableType>,
     vector<ComplexNonTriviallyRelocatableType>, vector<ComplexTriviallyRelocatableType>,
     vector<ComplexNonTriviallyRelocatableType, std::allocator<ComplexNonTriviallyRelocatableType>>,
@@ -509,6 +509,37 @@ TEST(VectorTest, SmallVectorOptimizedSizeInt) {
   EXPECT_EQ(ints, SmallInt16SmallVector({42, -56, -56, 42, 42, 37, 7567}));
 }
 
+template <typename T>
+class VectorTestUnalignedStorage : public ::testing::Test {
+ public:
+  using List = typename std::list<T>;
+};
+
+typedef ::testing::Types<SmallVector<UnalignedToPtr<3>, 5>, SmallVector<UnalignedToPtr<7>, 4>,
+                         SmallVector<UnalignedToPtr<5>, 3>, SmallVector<UnalignedToPtr2<3, uint16_t>, 4>,
+                         SmallVector<UnalignedToPtr2<7, uint16_t>, 3>, SmallVector<UnalignedToPtr2<5, uint16_t>, 2>,
+                         SmallVector<UnalignedToPtr2<3, uint32_t>, 3>, SmallVector<UnalignedToPtr2<7, uint32_t>, 2>,
+                         SmallVector<UnalignedToPtr2<5, uint32_t>, 1>>
+    UnalignedStorageTypes;
+TYPED_TEST_SUITE(VectorTestUnalignedStorage, UnalignedStorageTypes, );
+
+TYPED_TEST(VectorTestUnalignedStorage, SmallVectorUnalignedInlineStorage) {
+  using SmallVectorUnalignedType = TypeParam;
+  using VecOfVec = vector<SmallVectorUnalignedType>;
+  static_assert(std::alignment_of<SmallVectorUnalignedType>::value == 4);
+  SmallVectorUnalignedType v;
+  VecOfVec vecOfVec;
+  std::vector<unsigned int> expectedValues;
+  for (unsigned int i = 0; i < 10U; ++i) {
+    v.push_back(i);
+    expectedValues.push_back(i);
+    vecOfVec.push_back(v);
+    EXPECT_EQ(v.size(), i + 1U);
+    EXPECT_GE(v.capacity(), v.size());
+    EXPECT_EQ(v, SmallVectorUnalignedType(expectedValues.begin(), expectedValues.end()));
+  }
+}
+
 #if defined(AMC_CXX20) || (!defined(_MSC_VER) && defined(AMC_CXX17))
 // Some earlier compilers may not be able to compile this
 // Indeed, it is only specified from C++17 that std::vector may compile with incomplete types