Bug 1311277 Part 2 - Add move assignment for LinkedListElement and LinkedList. draft
authorTing-Yu Lin <tlin@mozilla.com>
Wed, 19 Oct 2016 13:30:51 +0800
changeset 427904 fd317ce1e4073965ec8afba1366d66a7675a7203
parent 427422 201a5a770addea749b4dfab105676640e7e81496
child 427905 0bb8470f53174e4a1f8196f2433576846f1e1801
push id33159
push userbmo:tlin@mozilla.com
push dateFri, 21 Oct 2016 03:26:06 +0000
bugs1311277
milestone52.0a1
Bug 1311277 Part 2 - Add move assignment for LinkedListElement and LinkedList. 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;
 }