Bug 1571759 - add nsTArray::EmplaceBack; r=erahm
authorNathan Froyd <froydnj@mozilla.com>
Fri, 16 Aug 2019 00:18:57 +0000
changeset 488387 eeb6741275a1d04cb23e16751f2670571f3b8ab5
parent 488386 f7db69e3462cf046fca6782483ec999bd997f982
child 488388 a080eb120278775f4d3df680ad6656be2250a882
push id113908
push userccoroiu@mozilla.com
push dateFri, 16 Aug 2019 09:57:53 +0000
treeherdermozilla-inbound@83fad6abe38a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerserahm
bugs1571759
milestone70.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 1571759 - add nsTArray::EmplaceBack; r=erahm Much preferred to `AppendElement(T(...))`. Differential Revision: https://phabricator.services.mozilla.com/D42024
xpcom/ds/nsCOMArray.cpp
xpcom/ds/nsTArray.h
xpcom/string/nsTString.h
xpcom/tests/gtest/TestTArray.cpp
--- a/xpcom/ds/nsCOMArray.cpp
+++ b/xpcom/ds/nsCOMArray.cpp
@@ -23,16 +23,21 @@ class nsTArrayElementTraits<nsISupports*
   static inline void Construct(E* aE) {
     new (mozilla::KnownNotNull, static_cast<void*>(aE)) E();
   }
   // Invoke the copy-constructor in place.
   template <class A>
   static inline void Construct(E* aE, const A& aArg) {
     new (mozilla::KnownNotNull, static_cast<void*>(aE)) E(aArg);
   }
+  // Construct in place.
+  template <class... Args>
+  static inline void Emplace(E* aE, Args&&... aArgs) {
+    new (mozilla::KnownNotNull, static_cast<void*>(aE)) E(std::forward<Args>(aArgs)...);
+  }
   // Invoke the destructor in place.
   static inline void Destruct(E* aE) { aE->~E(); }
 };
 
 static void ReleaseObjects(nsTArray<nsISupports*>& aArray);
 
 // implementations of non-trivial methods in nsCOMArray_base
 
--- a/xpcom/ds/nsTArray.h
+++ b/xpcom/ds/nsTArray.h
@@ -490,16 +490,35 @@ class nsTArray_base {
   // null.  If the array is empty, then this will point to sEmptyTArrayHeader.
   Header* mHdr;
 
   Header* Hdr() const { return mHdr; }
   Header** PtrToHdr() { return &mHdr; }
   static Header* EmptyHdr() { return &sEmptyTArrayHeader; }
 };
 
+namespace detail {
+
+// Used for argument checking in nsTArrayElementTraits::Emplace.
+template <typename... T>
+struct ChooseFirst;
+
+template <> struct ChooseFirst<> {
+  // Choose a default type that is guaranteed to not match E* for any
+  // nsTArray<E>.
+  typedef void Type;
+};
+
+template <typename A, typename... Args>
+struct ChooseFirst<A, Args...> {
+  typedef A Type;
+};
+
+}
+
 //
 // This class defines convenience functions for element specific operations.
 // Specialize this template if necessary.
 //
 template <class E>
 class nsTArrayElementTraits {
  public:
   // Invoke the default constructor in place.
@@ -516,16 +535,26 @@ class nsTArrayElementTraits {
   static inline void Construct(E* aE, A&& aArg) {
     typedef typename mozilla::RemoveCV<E>::Type E_NoCV;
     typedef typename mozilla::RemoveCV<A>::Type A_NoCV;
     static_assert(!mozilla::IsSame<E_NoCV*, A_NoCV>::value,
                   "For safety, we disallow constructing nsTArray<E> elements "
                   "from E* pointers. See bug 960591.");
     new (static_cast<void*>(aE)) E(std::forward<A>(aArg));
   }
+  // Construct in place.
+  template <class... Args>
+  static inline void Emplace(E* aE, Args&&... aArgs) {
+    typedef typename mozilla::RemoveCV<E>::Type E_NoCV;
+    typedef typename mozilla::RemoveCV<typename ::detail::ChooseFirst<Args...>::Type>::Type A_NoCV;
+    static_assert(!mozilla::IsSame<E_NoCV*, A_NoCV>::value,
+                  "For safety, we disallow constructing nsTArray<E> elements "
+                  "from E* pointers. See bug 960591.");
+    new (static_cast<void*>(aE)) E(std::forward<Args>(aArgs)...);
+  }
   // Invoke the destructor in place.
   static inline void Destruct(E* aE) { aE->~E(); }
 };
 
 // The default comparator used by nsTArray
 template <class A, class B>
 class nsDefaultComparator {
  public:
@@ -1624,16 +1653,28 @@ class nsTArray_Impl
  public:
   template <class Item, class Allocator, typename ActualAlloc = Alloc>
   /* MOZ_MUST_USE */
   elem_type* AppendElements(nsTArray_Impl<Item, Allocator>&& aArray,
                             const mozilla::fallible_t&) {
     return AppendElements<Item, Allocator>(std::move(aArray));
   }
 
+  // Append a new element, constructed in place from the provided arguments.
+ protected:
+  template <class... Args, typename ActualAlloc = Alloc>
+  elem_type* EmplaceBack(Args&&... aItem);
+
+ public:
+  template <class... Args>
+  MOZ_MUST_USE
+  elem_type* EmplaceBack(Args&&... aArgs, mozilla::fallible_t&) {
+    return EmplaceBack<Args..., FallibleAlloc>(std::forward<Args>(aArgs)...);
+  }
+
   // Append a new element, move constructing if possible.
  protected:
   template <class Item, typename ActualAlloc = Alloc>
   elem_type* AppendElement(Item&& aItem);
 
  public:
   template <class Item>
   /* MOZ_MUST_USE */
@@ -2395,16 +2436,30 @@ auto nsTArray_Impl<E, Alloc>::AppendElem
     return nullptr;
   }
   elem_type* elem = Elements() + Length();
   elem_traits::Construct(elem, std::forward<Item>(aItem));
   this->mHdr->mLength += 1;
   return elem;
 }
 
+template <typename E, class Alloc>
+template <class... Args, typename ActualAlloc>
+auto nsTArray_Impl<E, Alloc>::EmplaceBack(Args&&... aArgs) -> elem_type* {
+  // Length() + 1 is guaranteed to not overflow, so EnsureCapacity is OK.
+  if (!ActualAlloc::Successful(this->template EnsureCapacity<ActualAlloc>(
+          Length() + 1, sizeof(elem_type)))) {
+    return nullptr;
+  }
+  elem_type* elem = Elements() + Length();
+  elem_traits::Emplace(elem, std::forward<Args>(aArgs)...);
+  this->mHdr->mLength += 1;
+  return elem;
+}
+
 template <typename E, typename Alloc>
 inline void ImplCycleCollectionUnlink(nsTArray_Impl<E, Alloc>& aField) {
   aField.Clear();
 }
 
 template <typename E, typename Alloc>
 inline void ImplCycleCollectionTraverse(
     nsCycleCollectionTraversalCallback& aCallback,
@@ -2456,16 +2511,17 @@ class nsTArray : public nsTArray_Impl<E,
   template <class Allocator>
   self_type& operator=(nsTArray_Impl<E, Allocator>&& aOther) {
     base_type::operator=(std::move(aOther));
     return *this;
   }
 
   using base_type::AppendElement;
   using base_type::AppendElements;
+  using base_type::EmplaceBack;
   using base_type::EnsureLengthAtLeast;
   using base_type::InsertElementAt;
   using base_type::InsertElementsAt;
   using base_type::InsertElementSorted;
   using base_type::ReplaceElementsAt;
   using base_type::SetCapacity;
   using base_type::SetLength;
 };
--- a/xpcom/string/nsTString.h
+++ b/xpcom/string/nsTString.h
@@ -633,16 +633,21 @@ class nsTArrayElementTraits<nsTAutoStrin
       Instead_Use_nsTArray_of<nsTString<T>>* aE) {
     return 0;
   }
   template <class A>
   static Dont_Instantiate_nsTArray_of<nsTAutoString<T>>* Construct(
       Instead_Use_nsTArray_of<nsTString<T>>* aE, const A& aArg) {
     return 0;
   }
+  template <class... Args>
+  static Dont_Instantiate_nsTArray_of<nsTAutoString<T>>* Construct(
+      Instead_Use_nsTArray_of<nsTString<T>>* aE, Args&&... aArgs) {
+    return 0;
+  }
   static Dont_Instantiate_nsTArray_of<nsTAutoString<T>>* Destruct(
       Instead_Use_nsTArray_of<nsTString<T>>* aE) {
     return 0;
   }
 };
 
 /**
  * getter_Copies support for adopting raw string out params that are
--- a/xpcom/tests/gtest/TestTArray.cpp
+++ b/xpcom/tests/gtest/TestTArray.cpp
@@ -191,16 +191,97 @@ TEST(TArray, CopyOverlappingBackwards)
 
   array.InsertElementsAt(0, rangeLength);
 
   for (uint32_t i = 0; i < initialLength; ++i) {
     ASSERT_EQ(destructionCounters[i], 1u);
   }
 }
 
+namespace {
+
+class E {
+public:
+  E() : mA(-1), mB(-2) { constructCount++; }
+  E(int a, int b) : mA(a), mB(b) { constructCount++; }
+  E(E&& aRhs)
+    : mA(aRhs.mA), mB(aRhs.mB) {
+    aRhs.mA = 0;
+    aRhs.mB = 0;
+    moveCount++;
+  }
+
+  E& operator=(E&& aRhs) {
+    mA = aRhs.mA;
+    aRhs.mA = 0;
+    mB = aRhs.mB;
+    aRhs.mB = 0;
+    moveCount++;
+    return *this;
+  }
+
+
+  int a() const { return mA; }
+  int b() const { return mB; }
+
+  E(const E&) = delete;
+  E& operator=(const E&) = delete;
+
+  static size_t constructCount;
+  static size_t moveCount;
+
+private:
+  int mA;
+  int mB;
+};
+
+size_t E::constructCount = 0;
+size_t E::moveCount = 0;
+
+}
+
+TEST(TArray, Emplace)
+{
+  nsTArray<E> array;
+  array.SetCapacity(20);
+
+  ASSERT_EQ(array.Length(), 0u);
+
+  for (int i = 0; i < 10; i++) {
+    E s(i, i * i);
+    array.AppendElement(std::move(s));
+  }
+
+  ASSERT_EQ(array.Length(), 10u);
+  ASSERT_EQ(E::constructCount, 10u);
+  ASSERT_EQ(E::moveCount, 10u);
+
+  for (int i = 10; i < 20; i++) {
+    array.EmplaceBack(i, i * i);
+  }
+
+  ASSERT_EQ(array.Length(), 20u);
+  ASSERT_EQ(E::constructCount, 20u);
+  ASSERT_EQ(E::moveCount, 10u);
+
+  for (int i = 0; i < 20; i++) {
+    ASSERT_EQ(array[i].a(), i);
+    ASSERT_EQ(array[i].b(), i * i);
+  }
+
+  array.EmplaceBack();
+
+  ASSERT_EQ(array.Length(), 21u);
+  ASSERT_EQ(E::constructCount, 21u);
+  ASSERT_EQ(E::moveCount, 10u);
+
+  ASSERT_EQ(array[20].a(), -1);
+  ASSERT_EQ(array[20].b(), -2);
+}
+
 TEST(TArray, UnorderedRemoveElements)
 {
   // When removing an element from the end of the array, it can be removed in
   // place, by destroying it and decrementing the length.
   //
   // [ 1, 2, 3 ] => [ 1, 2 ]
   //         ^
   {