[PATCH] [libcxx] Properly convert the count arguments to the *_n algorithms before use.
Eric Fiselier
eric at efcs.ca
Tue Feb 10 08:06:12 PST 2015
Add inline comments.
================
Comment at: include/algorithm:1675
@@ -1674,2 +1674,3 @@
return _VSTD::__search_n<typename add_lvalue_reference<_BinaryPredicate>::type>
- (__first, __last, __count, __value_, __pred, typename iterator_traits<_ForwardIterator>::iterator_category());
+ (__first, __last, _VSTD::__convert_to_integral(__count), __value_,
+ __pred, typename iterator_traits<_ForwardIterator>::iterator_category());
----------------
mclow.lists wrote:
> I don't think you need a `_VSTD::` in front of `__convert_to_integral`, since it's not a legal identifier, so people can't define their own.
Acknowledged.
================
Comment at: include/type_traits:3663
@@ +3662,3 @@
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_ALWAYS_INLINE
+int __convert_to_integral(char __val) { return __val; }
+
----------------
mclow.lists wrote:
> Do you need cases for `signed char` and `unsigned char` ?
I thought you might if `char` was `signed char` and you wanted to convert `unsigned char x = -1;` but it actually seems you don't need the `char` overload at all.
I've updated the tests to check the conversion of both the min and max values for each type and I've removed the char overload.
================
Comment at: test/libcxx/type_traits/convert_to_integral.pass.cpp:73
@@ -68,1 +72,1 @@
}
\ No newline at end of file
----------------
mclow.lists wrote:
> As long as you're modifying this file, please add a newline at the end.
Acknowledged.
================
Comment at: test/std/algorithms/alg.modifying.operations/alg.copy/copy_n.pass.cpp:21
@@ -20,1 +20,3 @@
+struct IntegralConvertible
+{
----------------
mclow.lists wrote:
> Rather than having this `IntegralConvertible` in several source files, should it go into test/support?
>
> (suggestion, not requirement)
Acknowledged.
http://reviews.llvm.org/D7449
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the cfe-commits
mailing list