[PATCH] SemaLookup.cpp wrong boolean test

[Yaron Keren]

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/20131124/a7e10420/attachment.html>