On Jul 12, 2012, at 6:50 AM, John Mellor <joh...@chromium.org> wrote:
>
>
> To take an arbitrary example, lets say that while iterating through a
> ListHashSet something causes entries to be deleted. Intuitively it seems this
> needn't invalidate the iterator, as long as the entry the iterator is
> currently pointing to isn't removed. But is that actually the case in this
> particular implementation? A well-documented library like Java's
> LinkedHashSet will warn you "if the set is modified at any time after the
> iterator is created, in any way except through the iterator's own remove
> method, the iterator will throw a ConcurrentModificationException" and that's
> that. I just tried to find this out in WebKit and had to read though
> ListHashSetIterator, ListHashSetConstIterator, ListHashSetNode,
> ListHashSet::remove, ListHashSet::unlinkAndDelete, HashTable::remove,
> HashTable::internalCheckTableConsistency,
> HashTable::removeWithoutEntryConsistencyCheck,
> HashTable::removeAndInvalidateWithoutEntryConsistencyCheck,
> HashTable::invalidateIterators, HashTable::shrink, HashTable::rehash,
> ListHashSet::add, HashTable::add, HashTable::makeKnownGoodIterator,
> HashTableIterator, HashTable::AddResult and ListHashSetTranslator::translate,
> and even learn about using the template keyword for disambiguation.
> Eventually there was enough information to conclude that yes, it probably is
> safe since the ListHashSetNodes are allocated on the heap by
> ListHashSetTranslator::translate, so even though the HashTable invalidates
> its own iterators and HashTable::rehash may reallocate its storage, the
> actual ListHashSetNode pointed to by ListHashSetConstInterator should
> continue to exist. But constantly having to do such deep research makes
> coding highly inefficient, and there's a high risk of making errors.
I think documenting the public interfaces of core data structures is probably a
worthwhile tradeoff, since they change rarely but are used in many places. In
fact, ListHashSet already has a class comment and documents some method
behavior that is not obvious from the signature. Covering the issue you mention
seems worthwhile as well.
https://bugs.webkit.org/show_bug.cgi?id=91106
Regards,
Maciej
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev
