[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