[PATCH] [libcxx] Properly convert the count arguments to the *_n algorithms before use.
Marshall Clow
mclow.lists at gmail.com
Mon Feb 9 12:47:39 PST 2015
A few nits; but in general, this looks fine to me
================
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());
----------------
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.
================
Comment at: include/type_traits:3663
@@ +3662,3 @@
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_ALWAYS_INLINE
+int __convert_to_integral(char __val) { return __val; }
+
----------------
Do you need cases for `signed char` and `unsigned char` ?
================
Comment at: test/libcxx/type_traits/convert_to_integral.pass.cpp:73
@@ -68,1 +72,1 @@
}
\ No newline at end of file
----------------
As long as you're modifying this file, please add a newline at the end.
================
Comment at: test/std/algorithms/alg.modifying.operations/alg.copy/copy_n.pass.cpp:21
@@ -20,1 +20,3 @@
+struct IntegralConvertible
+{
----------------
Rather than having this `IntegralConvertible` in several source files, should it go into test/support?
(suggestion, not requirement)
http://reviews.llvm.org/D7449
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the cfe-commits
mailing list