[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