Bug 1287006 - Use alignas(). - r=waldo draft
authorJeff Gilbert <jgilbert@mozilla.com>
Fri, 06 Jan 2017 00:45:43 -0800
changeset 456845 deb639bb5b2526ccea9f624f5acf6b98f52e025e
parent 456716 a14094edbad78fc1d16e8d4c57902537cf286fd1
child 541337 22fcd273a30c48be41ccf616f36616b10bfe1bf2
push id40620
push userbmo:jgilbert@mozilla.com
push dateFri, 06 Jan 2017 10:13:29 +0000
reviewerswaldo
bugs1287006
milestone53.0a1
Bug 1287006 - Use alignas(). - r=waldo MozReview-Commit-ID: JrrCFWpS1VY
js/public/UbiNode.h
mfbt/Alignment.h
--- a/js/public/UbiNode.h
+++ b/js/public/UbiNode.h
@@ -368,21 +368,21 @@ class StackFrame {
         return *this;
     }
 
     // Because StackFrame is just a vtable pointer and an instance pointer, we
     // can memcpy everything around instead of making concrete classes define
     // virtual constructors. See the comment above Node's copy constructor for
     // more details; that comment applies here as well.
     StackFrame(const StackFrame& rhs) {
-        memcpy(storage.u.mBytes, rhs.storage.u.mBytes, sizeof(storage.u));
+        storage.memcpyFrom(rhs.storage);
     }
 
     StackFrame& operator=(const StackFrame& rhs) {
-        memcpy(storage.u.mBytes, rhs.storage.u.mBytes, sizeof(storage.u));
+        storage.memcpyFrom(rhs.storage);
         return *this;
     }
 
     bool operator==(const StackFrame& rhs) const { return base()->ptr == rhs.base()->ptr; }
     bool operator!=(const StackFrame& rhs) const { return !(*this == rhs); }
 
     explicit operator bool() const {
         return base()->ptr != nullptr;
@@ -722,21 +722,21 @@ class Node {
     // instances contain nothing but a vtable pointer and a data pointer.
     //
     // To be completely correct, concrete classes could provide a virtual
     // 'construct' member function, which we could invoke on rhs to construct an
     // instance in our storage. But this is good enough; there's no need to jump
     // through vtables for copying and assignment that are just going to move
     // two words around. The compiler knows how to optimize memcpy.
     Node(const Node& rhs) {
-        memcpy(storage.u.mBytes, rhs.storage.u.mBytes, sizeof(storage.u));
+        storage.memcpyFrom(rhs.storage);
     }
 
     Node& operator=(const Node& rhs) {
-        memcpy(storage.u.mBytes, rhs.storage.u.mBytes, sizeof(storage.u));
+        storage.memcpyFrom(rhs.storage);
         return *this;
     }
 
     bool operator==(const Node& rhs) const { return *base() == *rhs.base(); }
     bool operator!=(const Node& rhs) const { return *base() != *rhs.base(); }
 
     explicit operator bool() const {
         return base()->ptr != nullptr;
--- a/mfbt/Alignment.h
+++ b/mfbt/Alignment.h
@@ -105,50 +105,61 @@ struct AlignedElem<16>
  * to be extended one day...
  *
  * As an important side effect, pulling the storage into this template is
  * enough obfuscation to confuse gcc's strict-aliasing analysis into not giving
  * false negatives when we cast from the char buffer to whatever type we've
  * constructed using the bytes.
  */
 template<size_t Nbytes>
-struct AlignedStorage
+struct AlignedStorage final
 {
-  union U
-  {
-    char mBytes[Nbytes];
-    uint64_t mDummy;
-  } u;
+private:
+  // Note that we don't align to 16 here.
+  // Allowing it would generate warnings that `class alignas(8) js::DispatchWrapper` is
+  // actually aligned to 16, and will ignore its alignas(8). (C4359 on msvc)
+  alignas(Nbytes <= 1 ? 1 :
+           (Nbytes <= 2 ? 2 :
+             (Nbytes <= 4 ? 4 : 8)
+           )
+         ) char mBytes[Nbytes];
 
-  const void* addr() const { return u.mBytes; }
-  void* addr() { return u.mBytes; }
+public:
+  const void* addr() const { return mBytes; }
+  void* addr() { return mBytes; }
 
   AlignedStorage() = default;
 
   // AlignedStorage is non-copyable: the default copy constructor violates
   // strict aliasing rules, per bug 1269319.
   AlignedStorage(const AlignedStorage&) = delete;
   void operator=(const AlignedStorage&) = delete;
+
+  void memcpyFrom(const AlignedStorage& x) {
+    memcpy(mBytes, x.mBytes, Nbytes);
+  }
 };
 
 template<typename T>
-struct MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS AlignedStorage2
+struct MOZ_INHERIT_TYPE_ANNOTATIONS_FROM_TEMPLATE_ARGS AlignedStorage2 final
 {
-  union U
-  {
-    char mBytes[sizeof(T)];
-    uint64_t mDummy;
-  } u;
+private:
+  AlignedStorage<sizeof(T)> mStorage;
 
-  const T* addr() const { return reinterpret_cast<const T*>(u.mBytes); }
-  T* addr() { return static_cast<T*>(static_cast<void*>(u.mBytes)); }
+public:
+  const T* addr() const { return static_cast<const T*>(mStorage.addr()); }
+  T* addr() { return static_cast<T*>(mStorage.addr()); }
 
   AlignedStorage2() = default;
 
   // AlignedStorage2 is non-copyable: the default copy constructor violates
   // strict aliasing rules, per bug 1269319.
   AlignedStorage2(const AlignedStorage2&) = delete;
   void operator=(const AlignedStorage2&) = delete;
+
+  void memcpyFrom(const AlignedStorage2& x) {
+    mStorage.memcpyFrom(x.mStorage);
+  }
 };
 
 } /* namespace mozilla */
 
 #endif /* mozilla_Alignment_h */