[PATCH] SemaLookup.cpp wrong boolean test

[Yaron Keren]

Hi Alp,

Would you like me to continue with this count() patch for SmallSet and the
other bool count() functions in ADT?

Yaron



2013/11/24 Yaron Keren <yaron.keren at gmail.com>

> Hi,
>
> std::set count() indeed returns unsigned 0 or 1 only, so it makes sense
> for SmallSet to do the same.
>
> Regarding the patch,  the original code
>
>  return Set.count(V);
>
> should work without the ternary operator as it returns the std::set
> count() unsigned 0/1 result which is exactly what we want.
>
> Looking around in ADT,  some other count( are already patched like
> MapVector and StringMap while others are still "bool count(" :
>
>   DenseMap, DenseSet, ScopedHashTable, SmallPtrSet, SparseSet, ValueMap
>
> To be consistent with STL and the already-patched ADT, shouldn't we patch
> all of them?
>
> Diagnostics - the more the better, at least for me.
>
> Yaron
>
>
>
> 2013/11/24 Alp Toker <alp at nuanti.com>
>
>>
>> On 01/10/2013 19:25, Yaron Keren wrote:
>>
>>> In SemaLookup.cpp, line 4102 the test is somewhat wrong, as count
>>> (surprisingly given its name) return a boolean and not an integer.
>>>
>>> locs->second.count(TypoName.getLoc()) > 0)
>>>
>>> A patch is attached.
>>>
>>
>> Aaron fixed this in r192043.
>>
>>
>>
>>> I think this count function should be renamed to exists to better
>>> reflect its function.
>>>
>>
>> For better or worse count() is what STL templates expect, so best not to
>> change that.
>>
>> It could however return an unsigned integral type, and I've attached a
>> patch to do that, using unsigned rather than size_t to match
>> SmallSet::size(). LGTY?
>>
>> Also, MSVC diags on inequality comparison with bool, wonder if that would
>> be nice to have in clang (in fact I thought this kind of comparison range
>> check already existed, weird.)
>>
>> Alp.
>>
>>
>>
>>
>>
>>> Yaron
>>>
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>
>> --
>> http://www.nuanti.com
>> the browser experts
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131215/d801e06a/attachment.html>