[libcxx-commits] [libcxx] 12b01ab - [libc++] Don't trigger unsigned conversion warnings in std::advance

Louis Dionne via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jun 16 10:48:01 PDT 2020


Author: Louis Dionne
Date: 2020-06-16T13:47:47-04:00
New Revision: 12b01ab7fa10939d67ac7cb2da1d3ca8a41b5fcd

URL: https://github.com/llvm/llvm-project/commit/12b01ab7fa10939d67ac7cb2da1d3ca8a41b5fcd
DIFF: https://github.com/llvm/llvm-project/commit/12b01ab7fa10939d67ac7cb2da1d3ca8a41b5fcd.diff

LOG: [libc++] Don't trigger unsigned conversion warnings in std::advance

The Standard documents the signature of std::advance as

    template <class Iter, class Distance>
    constexpr void advance(Iter& i, Distance n);

Furthermore, it does not appear to put any restriction on what the type
of Distance should be. While it is understood that it should usually
be std::iterator_traits::difference_type, I couldn't find any wording
that mandates that. Similarly, I couldn't find wording that forces the
distance to be a signed type.

This patch changes std::advance to accept any type in the second argument,
which appears to be what the Standard mandates. We then coerce it to the
iterator's difference type, but that's an implementation detail.

Differential Revision: https://reviews.llvm.org/D81425

Added: 
    

Modified: 
    libcxx/include/iterator
    libcxx/test/std/iterators/iterator.primitives/iterator.operations/advance.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/include/iterator b/libcxx/include/iterator
index 57dd055b4ac9..a13214fca5e4 100644
--- a/libcxx/include/iterator
+++ b/libcxx/include/iterator
@@ -54,10 +54,8 @@ struct bidirectional_iterator_tag : public forward_iterator_tag       {};
 struct random_access_iterator_tag : public bidirectional_iterator_tag {};
 
 // 27.4.3, iterator operations
-// extension: second argument not conforming to C++03
-template <class InputIterator>  // constexpr in C++17
-  constexpr void advance(InputIterator& i,
-             typename iterator_traits<InputIterator>::
diff erence_type n);
+template <class InputIterator, class Distance>  // constexpr in C++17
+  constexpr void advance(InputIterator& i, Distance n);
 
 template <class InputIterator>  // constexpr in C++17
   constexpr typename iterator_traits<InputIterator>::
diff erence_type
@@ -663,13 +661,14 @@ void __advance(_RandIter& __i,
    __i += __n;
 }
 
-template <class _InputIter>
+template <class _InputIter, class _Distance>
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX14
-void advance(_InputIter& __i,
-             typename iterator_traits<_InputIter>::
diff erence_type __n)
+void advance(_InputIter& __i, _Distance __orig_n)
 {
-    _LIBCPP_ASSERT(__n >= 0 || __is_cpp17_bidirectional_iterator<_InputIter>::value,
-                       "Attempt to advance(it, -n) on a non-bidi iterator");
+    _LIBCPP_ASSERT(__orig_n >= 0 || __is_cpp17_bidirectional_iterator<_InputIter>::value,
+                   "Attempt to advance(it, n) with negative n on a non-bidirectional iterator");
+    typedef decltype(__convert_to_integral(__orig_n)) _IntegralSize;
+    _IntegralSize __n = __orig_n;
     __advance(__i, __n, typename iterator_traits<_InputIter>::iterator_category());
 }
 
@@ -711,7 +710,7 @@ next(_InputIter __x,
      typename iterator_traits<_InputIter>::
diff erence_type __n = 1)
 {
     _LIBCPP_ASSERT(__n >= 0 || __is_cpp17_bidirectional_iterator<_InputIter>::value,
-                       "Attempt to next(it, -n) on a non-bidi iterator");
+                       "Attempt to next(it, n) with negative n on a non-bidirectional iterator");
 
     _VSTD::advance(__x, __n);
     return __x;
@@ -728,7 +727,7 @@ prev(_InputIter __x,
      typename iterator_traits<_InputIter>::
diff erence_type __n = 1)
 {
     _LIBCPP_ASSERT(__n <= 0 || __is_cpp17_bidirectional_iterator<_InputIter>::value,
-                       "Attempt to prev(it, +n) on a non-bidi iterator");
+                       "Attempt to prev(it, n) with a positive n on a non-bidirectional iterator");
     _VSTD::advance(__x, -__n);
     return __x;
 }

diff  --git a/libcxx/test/std/iterators/iterator.primitives/iterator.operations/advance.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/iterator.operations/advance.pass.cpp
index c0dd85c783f1..75b86e3d3e89 100644
--- a/libcxx/test/std/iterators/iterator.primitives/iterator.operations/advance.pass.cpp
+++ b/libcxx/test/std/iterators/iterator.primitives/iterator.operations/advance.pass.cpp
@@ -10,25 +10,29 @@
 
 //   All of these became constexpr in C++17
 //
-// template <InputIterator Iter>
-//   constexpr void advance(Iter& i, Iter::
diff erence_type n);
+// template <InputIterator Iter, class Distance>
+//   constexpr void advance(Iter& i, Distance n);
 //
-// template <BidirectionalIterator Iter>
-//   constexpr void advance(Iter& i, Iter::
diff erence_type n);
+// template <BidirectionalIterator Iter, class Distance>
+//   constexpr void advance(Iter& i, Distance n);
 //
-// template <RandomAccessIterator Iter>
-//   constexpr void advance(Iter& i, Iter::
diff erence_type n);
+// template <RandomAccessIterator Iter, class Distance>
+//   constexpr void advance(Iter& i, Distance n);
+
+// Make sure we catch forced conversions to the 
diff erence_type if they happen.
+// ADDITIONAL_COMPILER_FLAGS: -Wsign-conversion
 
 #include <iterator>
 #include <cassert>
+#include <cstddef>
 #include <type_traits>
 
 #include "test_macros.h"
 #include "test_iterators.h"
 
-template <class It>
+template <class Distance, class It>
 TEST_CONSTEXPR_CXX17
-void check_advance(It it, typename std::iterator_traits<It>::
diff erence_type n, It result)
+void check_advance(It it, Distance n, It result)
 {
     static_assert(std::is_same<decltype(std::advance(it, n)), void>::value, "");
     std::advance(it, n);
@@ -38,14 +42,21 @@ void check_advance(It it, typename std::iterator_traits<It>::
diff erence_type n,
 TEST_CONSTEXPR_CXX17 bool tests()
 {
     const char* s = "1234567890";
-    check_advance(input_iterator<const char*>(s), 10, input_iterator<const char*>(s+10));
-    check_advance(forward_iterator<const char*>(s), 10, forward_iterator<const char*>(s+10));
-    check_advance(bidirectional_iterator<const char*>(s+5), 5, bidirectional_iterator<const char*>(s+10));
-    check_advance(bidirectional_iterator<const char*>(s+5), -5, bidirectional_iterator<const char*>(s));
-    check_advance(random_access_iterator<const char*>(s+5), 5, random_access_iterator<const char*>(s+10));
-    check_advance(random_access_iterator<const char*>(s+5), -5, random_access_iterator<const char*>(s));
-    check_advance(s+5, 5, s+10);
-    check_advance(s+5, -5, s);
+    typedef std::iterator_traits<const char*>::
diff erence_type Distance;
+    check_advance<Distance>(input_iterator<const char*>(s), 10, input_iterator<const char*>(s+10));
+    check_advance<Distance>(forward_iterator<const char*>(s), 10, forward_iterator<const char*>(s+10));
+    check_advance<Distance>(bidirectional_iterator<const char*>(s+5), 5, bidirectional_iterator<const char*>(s+10));
+    check_advance<Distance>(bidirectional_iterator<const char*>(s+5), -5, bidirectional_iterator<const char*>(s));
+    check_advance<Distance>(random_access_iterator<const char*>(s+5), 5, random_access_iterator<const char*>(s+10));
+    check_advance<Distance>(random_access_iterator<const char*>(s+5), -5, random_access_iterator<const char*>(s));
+    check_advance<Distance>(s+5, 5, s+10);
+    check_advance<Distance>(s+5, -5, s);
+
+    // Also check with other distance types
+    check_advance<std::size_t>(input_iterator<const char*>(s), 10u, input_iterator<const char*>(s+10));
+    check_advance<std::size_t>(forward_iterator<const char*>(s), 10u, forward_iterator<const char*>(s+10));
+    check_advance<std::size_t>(bidirectional_iterator<const char*>(s), 10u, bidirectional_iterator<const char*>(s+10));
+    check_advance<std::size_t>(random_access_iterator<const char*>(s), 10u, random_access_iterator<const char*>(s+10));
 
     return true;
 }


        


More information about the libcxx-commits mailing list