[PATCH] RFC: fail-fast iterators for DenseMap

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


> On 2015 Mar 2, at 19:49, Sanjoy Das <sanjoy at playingwithpointers.com> wrote:
> 
> As far as I can tell, DeclContext::lookups [1] tries to create an
> empty range using two default constructed
> DeclContext::all_lookups_iterator[2].
> DeclContext::all_lookups_iterator contains DenseMapIterators.
> 
> [1]: https://github.com/llvm-mirror/clang/blob/master/include/clang/AST/DeclLookups.h#L81
> [2]: https://github.com/llvm-mirror/clang/blob/master/include/clang/AST/DeclLookups.h#L35
> 

Comparing two default constructed iterators should be valid, but
I don't understand how your patch affects that operation.  Am I
missing something obvious?

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