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

Source/wtf/ListHashSet.h

 /*
  * Copyright (C) 2005, 2006, 2007, 2008, 2011, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2011, Benjamin Poulain <ikipou@gmail.com>
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
  *
  * This library is distributed in the hope that it will be useful,
@@ skipping 64 matching lines @@
         typedef ValueArg ValueType;
 
         typedef ListHashSetIterator<ValueType, inlineCapacity, HashArg> iterator;
         typedef ListHashSetConstIterator<ValueType, inlineCapacity, HashArg> const_iterator;
         friend class ListHashSetConstIterator<ValueType, inlineCapacity, HashArg>;
 
         typedef ListHashSetReverseIterator<ValueType, inlineCapacity, HashArg> reverse_iterator;
         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&);
         ListHashSet& operator=(const ListHashSet&);
         ~ListHashSet();
 
         void swap(ListHashSet&);
 
         unsigned size() const;
         unsigned capacity() const;
@@ skipping 633 matching lines @@
         return m_impl.template contains<BaseTranslator>(value);
     }
 
     template<typename T, size_t inlineCapacity, typename U>
     typename ListHashSet<T, inlineCapacity, U>::AddResult ListHashSet<T, inlineCapacity, U>::add(const ValueType &value)
     {
         createAllocatorIfNeeded();
         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>
     typename ListHashSet<T, inlineCapacity, U>::AddResult ListHashSet<T, inlineCapacity, U>::appendOrMoveToLast(const ValueType &value)
     {
         createAllocatorIfNeeded();
         typename ImplType::AddResult result = m_impl.template add<BaseTranslator>(value, m_allocator.get());
         Node* node = *result.iterator;
         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>
     typename ListHashSet<T, inlineCapacity, U>::AddResult ListHashSet<T, inlineCapacity, U>::prependOrMoveToFirst(const ValueType &value)
     {
         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>
     typename ListHashSet<T, inlineCapacity, U>::AddResult ListHashSet<T, inlineCapacity, U>::insertBefore(iterator it, const ValueType& newValue)
     {
         createAllocatorIfNeeded();
         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>
     typename ListHashSet<T, inlineCapacity, U>::AddResult ListHashSet<T, inlineCapacity, U>::insertBefore(const ValueType& beforeValue, const ValueType& newValue)
     {
         createAllocatorIfNeeded();
         return insertBefore(find(beforeValue), newValue);
     }
 
     template<typename T, size_t inlineCapacity, typename U>
@@ skipping 147 matching lines @@
     inline void deleteAllValues(const ListHashSet<T, inlineCapacity, U>& collection)
     {
         deleteAllValues<true, typename ListHashSet<T, inlineCapacity, U>::ValueType>(collection.m_impl);
     }
 
 } // namespace WTF
 
 using WTF::ListHashSet;
 
 #endif /* WTF_ListHashSet_h */