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 <[email protected]>
>
> 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
>> [email protected]
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
> --
> http://www.nuanti.com
> the browser experts
>
>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
