[PATCH] RFC: fail-fast iterators for DenseMap

Duncan P. N. Exon Smith dexonsmith at apple.com
Mon Mar 2 20:20:31 PST 2015


> 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 `!=`.)

> 
> 
>> 
>> Regardless, I'd try building with ASan to shake out the problems...
>> 
>>> On Mon, Mar 2, 2015 at 7:21 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
>>>> 
>>>> 
>>>> On Tue, Mar 3, 2015 at 12:53 PM, Sanjoy Das <sanjoy at playingwithpointers.com>
>>>> wrote:
>>>>> 
>>>>> I'm about to revert this because it broke clang.
>>>>> 
>>>>> The reason it broke clang is that clang assumes a default constructed
>>>>> DenseMapIterator is comparable with another default constructed
>>>>> DenseMapIterator.
>>>> 
>>>> 
>>>> 
>>>> Where does this happen?
>>>> I'd really like to see an actual example of how this makes sense to do
>>>> 
>>>> i have no idea whether it's generally considered "normal" to do this, but
>>>> i'm actually really curious to see a case where it is the right thing to do
>>>> :)
>>>> 
>> 





More information about the llvm-commits mailing list