[PATCH] SemaLookup.cpp wrong boolean test

[Alp Toker]

Hi Yaron,

I guess having count() return an integer has its benefits too in terms 
of matching STL.

Making int count() forward to a bool contains() function may offer a 
more friendly interface while maintaining STL compatibility.

I remember having stronger feelings about this cleanup a few weeks but I 
think they've ebbed away, so I leave it in your hands. Agree that it's 
inconsistent to have both styles in the codebase.

Alp.



On 15/12/2013 07:49, Yaron Keren wrote:
> 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 
> <mailto: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 <mailto: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 <mailto:cfe-commits at cs.uiuc.edu>
>             http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>         -- 
>         http://www.nuanti.com
>         the browser experts
>
>

-- 
http://www.nuanti.com
the browser experts