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