Bug 1338374 - Use alignas/alignof, and don't use sizeof where alignof should have been used, in MaybeOneOf. r=froydnj
authorJeff Walden <jwalden@mit.edu>
Mon, 30 Jan 2017 15:56:05 -0800
changeset 342908 f8970aa3b2d621c962cc9fbc29de2b1ae54f6bfb
parent 342907 eb9b629b161aa55f8147a444c9d61c5b7b0b0028
child 342909 eb11f699091d1fa8ce5dc0ef671868feeb179778
push id37410
push usercbook@mozilla.com
push dateWed, 15 Feb 2017 11:59:42 +0000
treeherderautoland@4702737fef39 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1338374
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1338374 - Use alignas/alignof, and don't use sizeof where alignof should have been used, in MaybeOneOf. r=froydnj
mfbt/MaybeOneOf.h
--- a/mfbt/MaybeOneOf.h
+++ b/mfbt/MaybeOneOf.h
@@ -1,55 +1,76 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+/*
+ * A class storing one of two optional value types that supports in-place lazy
+ * construction.
+ */
+
 #ifndef mozilla_MaybeOneOf_h
 #define mozilla_MaybeOneOf_h
 
-#include "mozilla/Alignment.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/Move.h"
 #include "mozilla/TemplateLib.h"
 
-#include <new>    // For placement new
+#include <new> // for placement new
+#include <stddef.h> // for size_t
 
 namespace mozilla {
 
 /*
  * MaybeOneOf<T1, T2> is like Maybe, but it supports constructing either T1
  * or T2. When a MaybeOneOf<T1, T2> is constructed, it is |empty()|, i.e.,
  * no value has been constructed and no destructor will be called when the
  * MaybeOneOf<T1, T2> is destroyed. Upon calling |construct<T1>()| or
  * |construct<T2>()|, a T1 or T2 object will be constructed with the given
  * arguments and that object will be destroyed when the owning MaybeOneOf is
  * destroyed.
+ *
+ * Because MaybeOneOf must be aligned suitable to hold any value stored within
+ * it, and because |alignas| requirements don't affect platform ABI with respect
+ * to how parameters are laid out in memory, MaybeOneOf can't be used as the
+ * type of a function parameter.  Pass MaybeOneOf to functions by pointer or
+ * reference instead.
  */
 template<class T1, class T2>
-class MaybeOneOf
+class MOZ_NON_PARAM MaybeOneOf
 {
-  AlignedStorage<tl::Max<sizeof(T1), sizeof(T2)>::value> storage;
+  static constexpr size_t StorageAlignment =
+    tl::Max<alignof(T1), alignof(T2)>::value;
+  static constexpr size_t StorageSize =
+    tl::Max<sizeof(T1), sizeof(T2)>::value;
+
+  alignas(StorageAlignment) unsigned char storage[StorageSize];
+
+  // GCC fails due to -Werror=strict-aliasing if |storage| is directly cast to
+  // T*.  Indirecting through these functions addresses the problem.
+  void* data() { return storage; }
+  const void* data() const { return storage; }
 
   enum State { None, SomeT1, SomeT2 } state;
   template <class T, class Ignored = void> struct Type2State {};
 
   template <class T>
   T& as()
   {
     MOZ_ASSERT(state == Type2State<T>::result);
-    return *(T*)storage.addr();
+    return *static_cast<T*>(data());
   }
 
   template <class T>
   const T& as() const
   {
     MOZ_ASSERT(state == Type2State<T>::result);
-    return *(T*)storage.addr();
+    return *static_cast<const T*>(data());
   }
 
 public:
   MaybeOneOf() : state(None) {}
   ~MaybeOneOf() { destroyIfConstructed(); }
 
   MaybeOneOf(MaybeOneOf&& rhs)
     : state(None)
@@ -61,17 +82,17 @@ public:
       } else {
         construct<T2>(Move(rhs.as<T2>()));
         rhs.as<T2>().~T2();
       }
       rhs.state = None;
     }
   }
 
-  MaybeOneOf &operator=(MaybeOneOf&& rhs)
+  MaybeOneOf& operator=(MaybeOneOf&& rhs)
   {
     MOZ_ASSERT(this != &rhs, "Self-move is prohibited");
     this->~MaybeOneOf();
     new(this) MaybeOneOf(Move(rhs));
     return *this;
   }
 
   bool empty() const { return state == None; }
@@ -79,17 +100,17 @@ public:
   template <class T>
   bool constructed() const { return state == Type2State<T>::result; }
 
   template <class T, class... Args>
   void construct(Args&&... aArgs)
   {
     MOZ_ASSERT(state == None);
     state = Type2State<T>::result;
-    ::new (storage.addr()) T(Forward<Args>(aArgs)...);
+    ::new (data()) T(Forward<Args>(aArgs)...);
   }
 
   template <class T>
   T& ref()
   {
     return as<T>();
   }