[PATCH] RFC: fail-fast iterators for DenseMap

[Duncan P. N. Exon Smith]

> On 2015 Mar 2, at 20:32, Chandler Carruth <chandlerc at google.com> wrote:
> 
> 
> On Mon, Mar 2, 2015 at 8:20 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> > On 2015 Mar 2, at 20:12, Sanjoy Das <sanjoy at playingwithpointers.com> wrote:
> >
> >> Comparing two default constructed iterators should be valid, but
> >> I don't understand how your patch affects that operation.  Am I
> >> missing something obvious?
> >
> > In the current implementation, default constructed iterators have a
> > nullptr epoch address, so looking up their epoch segfaults.
> 
> What I don't see is where you change the comparison of `DenseMapIterator`
> to depend on the epoch.  AFAICT, you only modified `++`, `*`, and `->`,
> none of which should be called from `==` or `!=`.
> 
> >
> > In any case, the fix should be easy; I just wanted a clarification on
> > what the intended semantics are.
> >
> 
> For `==` and `!=, you shouldn't worry about whether the handle is in sync
> with the debug base -- if anything, you should just check that the two
> handles are pointing at the same debug base (and have the same epoch as
> each other).  (But I still don't see how you've modified `==` or `!=`.)
> 
> The comparison operators call operator->

(Sorry, should have just looked at the code myself.)

That's silly though.  They should just be:

    bool operator==(const ConstIterator &RHS) const { return Ptr == RHS.Ptr; }
    bool operator!=(const ConstIterator &RHS) const { return Ptr != RHS.Ptr; }

Calling `->` is needlessly complicated.