Commit 981a814a authored by yutak's avatar yutak Committed by Commit bot

Fix ListHashSet::AddResult storing a Node* instead of a ValueType*.

ListHashSet::add() and similar used to return an AddResult value
containing a pointer to ListHashSetNode, instead of an actual value
specified by the user. This is unintuitive, and the users are forced to
append "->m_value" to obtain a pointer to the object they've really
added.

This patch fixes this issue, and giving what the users usually expect.

Interestingly, no production code (outside of tests) made use of
storedValue. LinkedHashSet did not have this issue.

BUG=582349

Review URL: https://codereview.chromium.org/1813693002

Cr-Commit-Position: refs/heads/master@{#381680}
parent ea3920fa
...@@ -87,13 +87,18 @@ public: ...@@ -87,13 +87,18 @@ public:
friend class ListHashSetReverseIterator<ListHashSet>; friend class ListHashSetReverseIterator<ListHashSet>;
friend class ListHashSetConstReverseIterator<ListHashSet>; friend class ListHashSetConstReverseIterator<ListHashSet>;
template <typename ValueType> struct HashTableAddResult final { struct AddResult final {
STACK_ALLOCATED(); STACK_ALLOCATED();
HashTableAddResult(Node* storedValue, bool isNewEntry) : storedValue(storedValue), isNewEntry(isNewEntry) { } friend class ListHashSet<ValueArg, inlineCapacity, HashArg, AllocatorArg>;
Node* storedValue; AddResult(Node* node, bool isNewEntry)
: storedValue(&node->m_value)
, isNewEntry(isNewEntry)
, m_node(node) { }
ValueType* storedValue;
bool isNewEntry; bool isNewEntry;
private:
Node* m_node;
}; };
typedef HashTableAddResult<ValueType> AddResult;
ListHashSet(); ListHashSet();
ListHashSet(const ListHashSet&); ListHashSet(const ListHashSet&);
...@@ -841,7 +846,7 @@ typename ListHashSet<T, inlineCapacity, U, V>::AddResult ListHashSet<T, inlineCa ...@@ -841,7 +846,7 @@ typename ListHashSet<T, inlineCapacity, U, V>::AddResult ListHashSet<T, inlineCa
template <typename T, size_t inlineCapacity, typename U, typename V> template <typename T, size_t inlineCapacity, typename U, typename V>
typename ListHashSet<T, inlineCapacity, U, V>::iterator ListHashSet<T, inlineCapacity, U, V>::addReturnIterator(ValuePassInType value) typename ListHashSet<T, inlineCapacity, U, V>::iterator ListHashSet<T, inlineCapacity, U, V>::addReturnIterator(ValuePassInType value)
{ {
return makeIterator(add(value).storedValue); return makeIterator(add(value).m_node);
} }
template <typename T, size_t inlineCapacity, typename U, typename V> template <typename T, size_t inlineCapacity, typename U, typename V>
......
...@@ -560,7 +560,7 @@ TEST(ListHashSetTest, WithOwnPtr) ...@@ -560,7 +560,7 @@ TEST(ListHashSetTest, WithOwnPtr)
// AddResult in a separate scope to avoid assertion hit, // AddResult in a separate scope to avoid assertion hit,
// since we modify the container further. // since we modify the container further.
OwnPtrSet::AddResult res1 = set.add(adoptPtr(ptr1)); OwnPtrSet::AddResult res1 = set.add(adoptPtr(ptr1));
EXPECT_EQ(res1.storedValue->m_value.get(), ptr1); EXPECT_EQ(res1.storedValue->get(), ptr1);
} }
EXPECT_FALSE(deleted1); EXPECT_FALSE(deleted1);
...@@ -572,7 +572,7 @@ TEST(ListHashSetTest, WithOwnPtr) ...@@ -572,7 +572,7 @@ TEST(ListHashSetTest, WithOwnPtr)
Dummy* ptr2 = new Dummy(deleted2); Dummy* ptr2 = new Dummy(deleted2);
{ {
OwnPtrSet::AddResult res2 = set.add(adoptPtr(ptr2)); OwnPtrSet::AddResult res2 = set.add(adoptPtr(ptr2));
EXPECT_EQ(res2.storedValue->m_value.get(), ptr2); EXPECT_EQ(res2.storedValue->get(), ptr2);
} }
EXPECT_FALSE(deleted2); EXPECT_FALSE(deleted2);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment