[PATCH] RFC: fail-fast iterators for DenseMap

[Duncan P. N. Exon Smith]

> On 2015 Feb 26, at 22:29, Sanjoy Das <sanjoy at playingwithpointers.com> wrote:
> 
> Hi ruiu, dexonsmith, dberlin,
> 
> This patch is an attempt at making `DenseMapIterator`s "fail-fast".
> Fail-fast iterators that have been invalidated due to insertion into
> the host `DenseMap` deterministically trip an assert (in debug mode)
> on access, instead of non-deterministically hitting memory corruption
> issues.
> 
> (This exact same thing can be done for `SmallPtrSet`s, but that is a
> later change.)
> 
> This change is fairly rough at the moment, but I want to get some
> early feedback on the approach and style before I spend too much time
> on this.
> 
> This change, in its current form, has already helped me fix two
> existing bugs:
> 
> http://reviews.llvm.org/rL230718
> http://reviews.llvm.org/rL230719
> 
> http://reviews.llvm.org/D7931
> 

+chandlerc, since we had a discussion about SmallPtrSet a few months
ago.

I'm in favour of this.

Morever, I'd go further.  We should bump the epoch on `DenseMap`s and
`SmallPtrSet`s on `erase()` calls as well.

This would break a lot of existing code, so it should be as a second
step, but the code would be fixed by deleting the generally unsafe:

    void erase(iterator);

in favour of the safer:

    iterator erase(iterator);

and updating code that looks correct (but isn't):

    for (auto I = M.begin(), E = M.end(); I != E;) {
      auto Current = I++;
      if (foo(*Current))
        M.erase(Current); // Or: M.erase(*Current);
    }

to the safer:

    for (auto I = M.begin(), E = M.end(); I != E;) {
      if (foo(*I))
        I = M.erase(I);
      else
        ++I;
    }

Note that the former version is *incorrect* in `SmallPtrSet`,
since in small mode this will skip over an element in the
iteration (since `SmallPtrSetImplBase::erase_imp()` swaps the
last element with the erased one).


> Files:
>  include/llvm/ADT/ADTModCountTracker.h
>  include/llvm/ADT/DenseMap.h
> 
> EMAIL PREFERENCES
>  http://reviews.llvm.org/settings/panel/emailpreferences/
> <D7931.20826.patch>