(Concept patch) Simplify WTF::HashTable::add() return value for size and performance

Source/wtf/ListHashSet.h

Index: Source/wtf/ListHashSet.h
diff --git a/Source/wtf/ListHashSet.h b/Source/wtf/ListHashSet.h
index 69e1c911cd89d72cc3e7564bc3f1647cb9e659a8..53fe56c4c544a9eee5d25122cd0ee93fac03472f 100644
--- a/Source/wtf/ListHashSet.h
+++ b/Source/wtf/ListHashSet.h
@@ -82,7 +82,12 @@ namespace WTF {
         typedef ListHashSetConstReverseIterator<ValueType, inlineCapacity, HashArg> const_reverse_iterator;
         friend class ListHashSetConstReverseIterator<ValueType, inlineCapacity, HashArg>;
 
-        typedef HashTableAddResult<iterator> AddResult;
+        template<typename ValueType> struct HashTableAddResult {
+        HashTableAddResult(void* iterator, bool isNewEntry) : iterator(iterator), isNewEntry(isNewEntry) { }
+            void* iterator;
+            bool isNewEntry;
+        };
+        typedef HashTableAddResult<ValueType> AddResult;
 
         ListHashSet();
         ListHashSet(const ListHashSet&);
@@ -736,7 +741,7 @@ namespace WTF {
         typename ImplType::AddResult result = m_impl.template add<BaseTranslator>(value, m_allocator.get());
         if (result.isNewEntry)
             appendNode(*result.iterator);
-        return AddResult(makeIterator(*result.iterator), result.isNewEntry);
+        return AddResult(result.iterator, result.isNewEntry);
     }
 
     template<typename T, size_t inlineCapacity, typename U>
@@ -748,7 +753,7 @@ namespace WTF {
         if (!result.isNewEntry)
             unlink(node);
         appendNode(node);
-        return AddResult(makeIterator(*result.iterator), result.isNewEntry);
+        return AddResult(result.iterator, result.isNewEntry);
     }
 
     template<typename T, size_t inlineCapacity, typename U>
@@ -756,11 +761,11 @@ namespace WTF {
     {
         createAllocatorIfNeeded();
         typename ImplType::AddResult result = m_impl.template add<BaseTranslator>(value, m_allocator.get());
-        Node* node = *result.iterator;
+        Node* node = result.iterator;

[Mikhail, 2014/02/04 09:57:41]

IMO the semantic is incorrect: iterator should not be the same thing as pointer.

[eseidel, 2014/02/05 22:17:20]

It's just the naming which is wrong.  If this was result.value it seems it would
read fine.

         if (!result.isNewEntry)
             unlink(node);
         prependNode(node);
-        return AddResult(makeIterator(*result.iterator), result.isNewEntry);
+        return AddResult(result.iterator, result.isNewEntry);
     }
 
     template<typename T, size_t inlineCapacity, typename U>
@@ -770,7 +775,7 @@ namespace WTF {
         typename ImplType::AddResult result = m_impl.template add<BaseTranslator>(newValue, m_allocator.get());
         if (result.isNewEntry)
             insertNodeBefore(it.node(), *result.iterator);
-        return AddResult(makeIterator(*result.iterator), result.isNewEntry);
+        return AddResult(result.iterator, result.isNewEntry);
     }
 
     template<typename T, size_t inlineCapacity, typename U>