Bug 1311277 Part 2 - Add move assignment for LinkedListElement and LinkedList. r=froydnj
authorTing-Yu Lin <tlin@mozilla.com>
Wed, 19 Oct 2016 13:30:51 +0800
changeset 364042 ba5d30bdad0d84ae6e1809a5946ea136a92dad44
parent 364041 8cf3c1b52fac16bb337dd2d0f99bf0d862d2b14b
child 364043 d6bd5ef77aebdab96355e022bf6ff1fefbf20ddf
push id1369
push userjlorenzo@mozilla.com
push dateMon, 27 Feb 2017 14:59:41 +0000
treeherdermozilla-release@d75a1dba431f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1311277
milestone52.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 1311277 Part 2 - Add move assignment for LinkedListElement and LinkedList. r=froydnj MozReview-Commit-ID: 7NjbxamX87U
mfbt/LinkedList.h
mfbt/tests/TestLinkedList.cpp
--- a/mfbt/LinkedList.h
+++ b/mfbt/LinkedList.h
@@ -120,44 +120,33 @@ private:
 
 public:
   LinkedListElement()
     : mNext(this),
       mPrev(this),
       mIsSentinel(false)
   { }
 
+  /*
+   * Moves |aOther| into |*this|. If |aOther| is already in a list, then
+   * |aOther| is removed from the list and replaced by |*this|.
+   */
   LinkedListElement(LinkedListElement<T>&& aOther)
     : mIsSentinel(aOther.mIsSentinel)
   {
-    if (!aOther.isInList()) {
-      mNext = this;
-      mPrev = this;
-      return;
-    }
-
-    MOZ_ASSERT(aOther.mNext->mPrev == &aOther);
-    MOZ_ASSERT(aOther.mPrev->mNext == &aOther);
+    adjustLinkForMove(Move(aOther));
+  }
 
-    /*
-     * Initialize |this| with |aOther|'s mPrev/mNext pointers, and adjust those
-     * element to point to this one.
-     */
-    mNext = aOther.mNext;
-    mPrev = aOther.mPrev;
-
-    mNext->mPrev = this;
-    mPrev->mNext = this;
-
-    /*
-     * Adjust |aOther| so it doesn't think it's in a list.  This makes it
-     * safely destructable.
-     */
-    aOther.mNext = &aOther;
-    aOther.mPrev = &aOther;
+  LinkedListElement& operator=(LinkedListElement<T>&& aOther)
+  {
+    MOZ_ASSERT(mIsSentinel == aOther.mIsSentinel, "Mismatch NodeKind!");
+    MOZ_ASSERT(!isInList(),
+               "Assigning to an element in a list messes up that list!");
+    adjustLinkForMove(Move(aOther));
+    return *this;
   }
 
   ~LinkedListElement()
   {
     if (!mIsSentinel && isInList()) {
       remove();
     }
   }
@@ -282,17 +271,49 @@ private:
     MOZ_ASSERT(!listElem->isInList());
 
     listElem->mNext = this;
     listElem->mPrev = this->mPrev;
     this->mPrev->mNext = listElem;
     this->mPrev = listElem;
   }
 
-private:
+  /*
+   * Adjust mNext and mPrev for implementing move constructor and move
+   * assignment.
+   */
+  void adjustLinkForMove(LinkedListElement<T>&& aOther)
+  {
+    if (!aOther.isInList()) {
+      mNext = this;
+      mPrev = this;
+      return;
+    }
+
+    MOZ_ASSERT(aOther.mNext->mPrev == &aOther);
+    MOZ_ASSERT(aOther.mPrev->mNext == &aOther);
+
+    /*
+     * Initialize |this| with |aOther|'s mPrev/mNext pointers, and adjust those
+     * element to point to this one.
+     */
+    mNext = aOther.mNext;
+    mPrev = aOther.mPrev;
+
+    mNext->mPrev = this;
+    mPrev->mNext = this;
+
+    /*
+     * Adjust |aOther| so it doesn't think it's in a list.  This makes it
+     * safely destructable.
+     */
+    aOther.mNext = &aOther;
+    aOther.mPrev = &aOther;
+  }
+
   LinkedListElement& operator=(const LinkedListElement<T>& aOther) = delete;
   LinkedListElement(const LinkedListElement<T>& aOther) = delete;
 };
 
 template<typename T>
 class LinkedList
 {
 private:
@@ -320,16 +341,23 @@ public:
   };
 
   LinkedList() : sentinel(LinkedListElement<T>::NODE_KIND_SENTINEL) { }
 
   LinkedList(LinkedList<T>&& aOther)
     : sentinel(mozilla::Move(aOther.sentinel))
   { }
 
+  LinkedList& operator=(LinkedList<T>&& aOther)
+  {
+    MOZ_ASSERT(isEmpty(), "Assigning to a non-empty list leaks elements in that list!");
+    sentinel = mozilla::Move(aOther.sentinel);
+    return *this;
+  }
+
   ~LinkedList() {
     MOZ_ASSERT(isEmpty(),
                "failing this assertion means this LinkedList's creator is "
                "buggy: it should have removed all this list's elements before "
                "the list's destruction");
   }
 
   /*
--- a/mfbt/tests/TestLinkedList.cpp
+++ b/mfbt/tests/TestLinkedList.cpp
@@ -7,17 +7,17 @@
 #include "mozilla/Assertions.h"
 #include "mozilla/LinkedList.h"
 
 using mozilla::LinkedList;
 using mozilla::LinkedListElement;
 
 struct SomeClass : public LinkedListElement<SomeClass> {
   unsigned int mValue;
-  explicit SomeClass(int aValue) : mValue(aValue) {}
+  explicit SomeClass(int aValue = 0) : mValue(aValue) {}
   void incr() { ++mValue; }
 };
 
 template <size_t N>
 static void
 CheckListValues(LinkedList<SomeClass>& list, unsigned int (&values)[N])
 {
   size_t count = 0;
@@ -29,17 +29,17 @@ CheckListValues(LinkedList<SomeClass>& l
 }
 
 static void
 TestList()
 {
   LinkedList<SomeClass> list;
 
   SomeClass one(1), two(2), three(3);
-  
+
   MOZ_RELEASE_ASSERT(list.isEmpty());
   MOZ_RELEASE_ASSERT(!list.getFirst());
   MOZ_RELEASE_ASSERT(!list.getLast());
   MOZ_RELEASE_ASSERT(!list.popFirst());
   MOZ_RELEASE_ASSERT(!list.popLast());
 
   for (SomeClass* x : list) {
     MOZ_RELEASE_ASSERT(x);
@@ -47,17 +47,17 @@ TestList()
   }
 
   list.insertFront(&one);
   { unsigned int check[] { 1 }; CheckListValues(list, check); }
 
   MOZ_RELEASE_ASSERT(one.isInList());
   MOZ_RELEASE_ASSERT(!two.isInList());
   MOZ_RELEASE_ASSERT(!three.isInList());
-  
+
   MOZ_RELEASE_ASSERT(!list.isEmpty());
   MOZ_RELEASE_ASSERT(list.getFirst()->mValue == 1);
   MOZ_RELEASE_ASSERT(list.getLast()->mValue == 1);
 
   list.insertFront(&two);
   { unsigned int check[] { 2, 1 }; CheckListValues(list, check); }
 
   MOZ_RELEASE_ASSERT(list.getFirst()->mValue == 2);
@@ -112,16 +112,55 @@ TestList()
   }
 
   MOZ_RELEASE_ASSERT(list.getFirst() == &two);
   MOZ_RELEASE_ASSERT(list.getLast() == &three);
   MOZ_RELEASE_ASSERT(list.getFirst()->mValue == 3);
   MOZ_RELEASE_ASSERT(list.getLast()->mValue == 4);
 }
 
+static void
+TestMove()
+{
+  auto MakeSomeClass =
+    [] (unsigned int aValue) -> SomeClass { return SomeClass(aValue); };
+
+  LinkedList<SomeClass> list1;
+
+  // Test move constructor for LinkedListElement.
+  SomeClass c1(MakeSomeClass(1));
+  list1.insertBack(&c1);
+
+  // Test move assignment for LinkedListElement from an element not in a
+  // list.
+  SomeClass c2;
+  c2 = MakeSomeClass(2);
+  list1.insertBack(&c2);
+
+  // Test move assignment of LinkedListElement from an element already in a
+  // list.
+  SomeClass c3;
+  c3 = Move(c2);
+  MOZ_RELEASE_ASSERT(!c2.isInList());
+  MOZ_RELEASE_ASSERT(c3.isInList());
+
+  // Test move constructor for LinkedList.
+  LinkedList<SomeClass> list2(Move(list1));
+  { unsigned int check[] { 1, 2 }; CheckListValues(list2, check); }
+  MOZ_RELEASE_ASSERT(list1.isEmpty());
+
+  // Test move assignment for LinkedList.
+  LinkedList<SomeClass> list3;
+  list3 = Move(list2);
+  { unsigned int check[] { 1, 2 }; CheckListValues(list3, check); }
+  MOZ_RELEASE_ASSERT(list2.isEmpty());
+
+  list3.clear();
+}
+
 struct PrivateClass : private LinkedListElement<PrivateClass> {
   friend class mozilla::LinkedList<PrivateClass>;
   friend class mozilla::LinkedListElement<PrivateClass>;
 };
 
 static void
 TestPrivate()
 {
@@ -138,10 +177,11 @@ TestPrivate()
   MOZ_RELEASE_ASSERT(count == 2);
 }
 
 int
 main()
 {
   TestList();
   TestPrivate();
+  TestMove();
   return 0;
 }