Bug 1478879 - Define Range/Enum in terms of Iterator/ModIterator. r=luke draft
authorNicholas Nethercote <nnethercote@mozilla.com>
Tue, 31 Jul 2018 10:23:03 +1000
changeset 825229 ceeb62ab2d38dd1db338b15c652b20cc4a6b99d0
parent 825228 a9645faaf496f726761b9e5e643c11b22a88b90a
child 825230 ff6d4414438f09f699d52e1758b76fb5c0d24fa6
push id118038
push usernnethercote@mozilla.com
push dateWed, 01 Aug 2018 02:34:08 +0000
reviewersluke
bugs1478879
milestone63.0a1
Bug 1478879 - Define Range/Enum in terms of Iterator/ModIterator. r=luke To reduce the code duplication. MozReview-Commit-ID: EVnPOYNJGwl
mfbt/HashTable.h
--- a/mfbt/HashTable.h
+++ b/mfbt/HashTable.h
@@ -1361,159 +1361,71 @@ public:
         mTable.compactIfUnderloaded();
       }
     }
   };
 
   // Range is similar to Iterator, but uses different terminology.
   class Range
   {
-  protected:
     friend class HashTable;
 
-    Range(const HashTable& aTable, Entry* aCur, Entry* aEnd)
-      : mCur(aCur)
-      , mEnd(aEnd)
-#ifdef DEBUG
-      , mTable(aTable)
-      , mMutationCount(aTable.mMutationCount)
-      , mGeneration(aTable.generation())
-      , mValidEntry(true)
-#endif
+    Iterator mIter;
+
+  protected:
+    explicit Range(const HashTable& table)
+      : mIter(table)
     {
-      while (mCur < mEnd && !mCur->isLive()) {
-        ++mCur;
-      }
     }
 
-    Entry* mCur;
-    Entry* mEnd;
-#ifdef DEBUG
-    const HashTable& mTable;
-    uint64_t mMutationCount;
-    Generation mGeneration;
-    bool mValidEntry;
-#endif
-
   public:
-    bool empty() const
-    {
-#ifdef DEBUG
-      MOZ_ASSERT(mGeneration == mTable.generation());
-      MOZ_ASSERT(mMutationCount == mTable.mMutationCount);
-#endif
-      return mCur == mEnd;
-    }
+    bool empty() const { return mIter.done(); }
 
-    T& front() const
-    {
-      MOZ_ASSERT(!empty());
-#ifdef DEBUG
-      MOZ_ASSERT(mValidEntry);
-      MOZ_ASSERT(mGeneration == mTable.generation());
-      MOZ_ASSERT(mMutationCount == mTable.mMutationCount);
-#endif
-      return mCur->get();
-    }
+    T& front() const { return mIter.get(); }
 
-    void popFront()
-    {
-      MOZ_ASSERT(!empty());
-#ifdef DEBUG
-      MOZ_ASSERT(mGeneration == mTable.generation());
-      MOZ_ASSERT(mMutationCount == mTable.mMutationCount);
-#endif
-      while (++mCur < mEnd && !mCur->isLive()) {
-        continue;
-      }
-#ifdef DEBUG
-      mValidEntry = true;
-#endif
-    }
+    void popFront() { return mIter.next(); }
   };
 
   // Enum is similar to ModIterator, but uses different terminology.
-  class Enum : public Range
+  class Enum
   {
-    friend class HashTable;
-
-    HashTable& mTable;
-    bool mRekeyed;
-    bool mRemoved;
+    ModIterator mIter;
 
     // Enum is movable but not copyable.
     Enum(const Enum&) = delete;
     void operator=(const Enum&) = delete;
 
   public:
     template<class Map>
     explicit Enum(Map& map)
-      : Range(map.all())
-      , mTable(map.mImpl)
-      , mRekeyed(false)
-      , mRemoved(false)
+      : mIter(map.mImpl)
+    {
+    }
+
+    MOZ_IMPLICIT Enum(Enum&& other)
+      : mIter(std::move(other.mIter))
     {
     }
 
-    MOZ_IMPLICIT Enum(Enum&& aOther)
-      : Range(aOther)
-      , mTable(aOther.mTable)
-      , mRekeyed(aOther.mRekeyed)
-      , mRemoved(aOther.mRemoved)
-    {
-      aOther.mRekeyed = false;
-      aOther.mRemoved = false;
-    }
+    bool empty() const { return mIter.done(); }
+
+    T& front() const { return mIter.get(); }
 
-    void removeFront()
-    {
-      mTable.remove(*this->mCur);
-      mRemoved = true;
-#ifdef DEBUG
-      this->mValidEntry = false;
-      this->mMutationCount = mTable.mMutationCount;
-#endif
-    }
+    void popFront() { return mIter.next(); }
 
-    NonConstT& mutableFront()
-    {
-      MOZ_ASSERT(!this->empty());
-#ifdef DEBUG
-      MOZ_ASSERT(this->mValidEntry);
-      MOZ_ASSERT(this->mGeneration == this->Range::mTable.generation());
-      MOZ_ASSERT(this->mMutationCount == this->Range::mTable.mMutationCount);
-#endif
-      return this->mCur->getMutable();
-    }
+    void removeFront() { mIter.remove(); }
+
+    NonConstT& mutableFront() { return mIter.getMutable(); }
 
     void rekeyFront(const Lookup& aLookup, const Key& aKey)
     {
-      MOZ_ASSERT(&aKey != &HashPolicy::getKey(this->mCur->get()));
-      Ptr p(*this->mCur, mTable);
-      mTable.rekeyWithoutRehash(p, aLookup, aKey);
-      mRekeyed = true;
-#ifdef DEBUG
-      this->mValidEntry = false;
-      this->mMutationCount = mTable.mMutationCount;
-#endif
+      mIter.rekey(aLookup, aKey);
     }
 
-    void rekeyFront(const Key& aKey) { rekeyFront(aKey, aKey); }
-
-    ~Enum()
-    {
-      if (mRekeyed) {
-        mTable.mGen++;
-        mTable.checkOverRemoved();
-      }
-
-      if (mRemoved) {
-        mTable.compactIfUnderloaded();
-      }
-    }
+    void rekeyFront(const Key& aKey) { mIter.rekey(aKey); }
   };
 
   // HashTable is movable
   HashTable(HashTable&& aRhs)
     : AllocPolicy(aRhs)
   {
     PodAssign(this, &aRhs);
     aRhs.mTable = nullptr;
@@ -2112,17 +2024,17 @@ public:
   {
     MOZ_ASSERT(mTable);
     return ModIterator(*this);
   }
 
   Range all() const
   {
     MOZ_ASSERT(mTable);
-    return Range(*this, mTable, mTable + capacity());
+    return Range(*this);
   }
 
   bool empty() const
   {
     MOZ_ASSERT(mTable);
     return !mEntryCount;
   }