Bug 1469003 - Convert SegmentedVector to use a manually aligned |unsigned char| array for storage, using a clean C++11 idiom. r=froydnj
authorJeff Walden <jwalden@mit.edu>
Mon, 18 Jun 2018 11:55:41 -0700
changeset 1541576 a61ae45785b06e24c58e02b8e84025583b0683bb
parent 1541575 d8eb78d6b65d8583281f22d84b5bc82a7c05ef98
child 1541577 b390ebb847ee506c501ea5b2ba2d3d90efabd79d
child 1541917 c53a24b48664f347036b8e3efa6d69ccdd1d4ead
push id278906
push useramarchesini@mozilla.com
push dateMon, 18 Jun 2018 21:43:51 +0000
treeherdertry@c3195ce5416b [default view] [failures only]
reviewersfroydnj
bugs1469003
milestone62.0a1
Bug 1469003 - Convert SegmentedVector to use a manually aligned |unsigned char| array for storage, using a clean C++11 idiom. r=froydnj
mfbt/SegmentedVector.h
--- a/mfbt/SegmentedVector.h
+++ b/mfbt/SegmentedVector.h
@@ -15,19 +15,19 @@
 //
 // - In the case where you know the final size in advance and so can set the
 //   capacity appropriately, using SegmentedVector still avoids the need for
 //   large allocations (which can trigger OOMs).
 
 #ifndef mozilla_SegmentedVector_h
 #define mozilla_SegmentedVector_h
 
-#include "mozilla/Alignment.h"
 #include "mozilla/AllocPolicy.h"
 #include "mozilla/Array.h"
+#include "mozilla/Attributes.h"
 #include "mozilla/LinkedList.h"
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/Move.h"
 #include "mozilla/TypeTraits.h"
 
 #include <new>  // for placement new
 
 namespace mozilla {
@@ -52,28 +52,39 @@ template<typename T,
          size_t IdealSegmentSize = 4096,
          typename AllocPolicy = MallocAllocPolicy>
 class SegmentedVector : private AllocPolicy
 {
   template<size_t SegmentCapacity>
   struct SegmentImpl
     : public mozilla::LinkedListElement<SegmentImpl<SegmentCapacity>>
   {
+  private:
+    uint32_t mLength;
+    alignas(T) MOZ_INIT_OUTSIDE_CTOR unsigned char mData[sizeof(T) * SegmentCapacity];
+
+    // Some versions of GCC treat it as a -Wstrict-aliasing violation (ergo a
+    // -Werror compile error) to reinterpret_cast<> |mData| to |T*|, even
+    // through |void*|.  Placing the latter cast in these separate functions
+    // breaks the chain such that affected GCC versions no longer warn/error.
+    void* RawData() { return mData; }
+
+  public:
     SegmentImpl() : mLength(0) {}
 
     ~SegmentImpl()
     {
       for (uint32_t i = 0; i < mLength; i++) {
         (*this)[i].~T();
       }
     }
 
     uint32_t Length() const { return mLength; }
 
-    T* Elems() { return reinterpret_cast<T*>(&mStorage.mBuf); }
+    T* Elems() { return reinterpret_cast<T*>(RawData()); }
 
     T& operator[](size_t aIndex)
     {
       MOZ_ASSERT(aIndex < mLength);
       return Elems()[aIndex];
     }
 
     const T& operator[](size_t aIndex) const
@@ -93,28 +104,16 @@ class SegmentedVector : private AllocPol
     }
 
     void PopLast()
     {
       MOZ_ASSERT(mLength > 0);
       (*this)[mLength - 1].~T();
       mLength--;
     }
-
-    uint32_t mLength;
-
-    // The union ensures that the elements are appropriately aligned.
-    union Storage
-    {
-      char mBuf[sizeof(T) * SegmentCapacity];
-      mozilla::AlignedElem<MOZ_ALIGNOF(T)> mAlign;
-    } mStorage;
-
-    static_assert(MOZ_ALIGNOF(T) == MOZ_ALIGNOF(Storage),
-                  "SegmentedVector provides incorrect alignment");
   };
 
   // See how many we elements we can fit in a segment of IdealSegmentSize. If
   // IdealSegmentSize is too small, it'll be just one. The +1 is because
   // kSingleElementSegmentSize already accounts for one element.
   static const size_t kSingleElementSegmentSize = sizeof(SegmentImpl<1>);
   static const size_t kSegmentCapacity =
     kSingleElementSegmentSize <= IdealSegmentSize