[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