[libcxx-commits] [libcxx] [libc++] Disallow std::prev(non_cpp17_bidi_iterator) (PR #112102)

via libcxx-commits libcxx-commits at lists.llvm.org
Sat Oct 19 02:21:21 PDT 2024


https://github.com/huixie90 updated https://github.com/llvm/llvm-project/pull/112102

>From 6cd8e5c2031bffab574b62e63bb8e7a7befe4779 Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Sat, 12 Oct 2024 18:24:36 +0100
Subject: [PATCH 1/4] [libc++] Disallow std::prev(non_cpp17_bidi_iterator)

---
 libcxx/include/__iterator/prev.h                  | 10 +++++++++-
 .../iterator.operations/prev.pass.cpp             | 15 +++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/libcxx/include/__iterator/prev.h b/libcxx/include/__iterator/prev.h
index 7e97203836eb98..47bfb089504818 100644
--- a/libcxx/include/__iterator/prev.h
+++ b/libcxx/include/__iterator/prev.h
@@ -17,6 +17,7 @@
 #include <__iterator/incrementable_traits.h>
 #include <__iterator/iterator_traits.h>
 #include <__type_traits/enable_if.h>
+#include <__utility/move.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -26,7 +27,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <class _InputIter, __enable_if_t<__has_input_iterator_category<_InputIter>::value, int> = 0>
 [[__nodiscard__]] inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX17 _InputIter
-prev(_InputIter __x, typename iterator_traits<_InputIter>::difference_type __n = 1) {
+prev(_InputIter __x, typename iterator_traits<_InputIter>::difference_type __n) {
   // Calling `advance` with a negative value on a non-bidirectional iterator is a no-op in the current implementation.
   // Note that this check duplicates the similar check in `std::advance`.
   _LIBCPP_ASSERT_PEDANTIC(__n <= 0 || __has_bidirectional_iterator_category<_InputIter>::value,
@@ -35,6 +36,13 @@ prev(_InputIter __x, typename iterator_traits<_InputIter>::difference_type __n =
   return __x;
 }
 
+template <class _BidirectionalIterator,
+          __enable_if_t<__has_bidirectional_iterator_category<_BidirectionalIterator>::value, int> = 0>
+[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX17 _BidirectionalIterator
+prev(_BidirectionalIterator __it) {
+  return std::prev(std::move(__it), 1);
+}
+
 #if _LIBCPP_STD_VER >= 20
 
 // [range.iter.op.prev]
diff --git a/libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp
index 784c1b93b7ca89..be0295d4f7c165 100644
--- a/libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp
+++ b/libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp
@@ -18,6 +18,21 @@
 #include "test_macros.h"
 #include "test_iterators.h"
 
+template <class Iter>
+std::false_type prev_test(...);
+
+template <class Iter>
+decltype((void) std::prev(std::declval<Iter>()), std::true_type()) prev_test(int);
+
+template <class Iter>
+using CanPrev = decltype(prev_test<Iter>(0));
+
+static_assert(!CanPrev<cpp17_input_iterator<int*> >::value, "");
+static_assert(CanPrev<bidirectional_iterator<int*> >::value, "");
+#if TEST_STD_VER >= 20
+    static_assert(!CanPrev<cpp20_random_access_iterator<int*> >::value);
+#endif
+
 template <class It>
 TEST_CONSTEXPR_CXX17 void
 check_prev_n(It it, typename std::iterator_traits<It>::difference_type n, It result)

>From 0ab15ce7aca36ceec6f2130d059dcffbc748c9c0 Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Sun, 13 Oct 2024 10:44:42 +0100
Subject: [PATCH 2/4] CI

---
 libcxx/include/__iterator/prev.h                             | 5 +++++
 .../iterator.primitives/iterator.operations/prev.pass.cpp    | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/libcxx/include/__iterator/prev.h b/libcxx/include/__iterator/prev.h
index 47bfb089504818..d709d0b6787d60 100644
--- a/libcxx/include/__iterator/prev.h
+++ b/libcxx/include/__iterator/prev.h
@@ -23,6 +23,9 @@
 #  pragma GCC system_header
 #endif
 
+_LIBCPP_PUSH_MACROS
+#include <__undef_macros>
+
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <class _InputIter, __enable_if_t<__has_input_iterator_category<_InputIter>::value, int> = 0>
@@ -78,4 +81,6 @@ inline constexpr auto prev = __prev{};
 
 _LIBCPP_END_NAMESPACE_STD
 
+_LIBCPP_POP_MACROS
+
 #endif // _LIBCPP___ITERATOR_PREV_H
diff --git a/libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp
index be0295d4f7c165..97b9aee7045396 100644
--- a/libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp
+++ b/libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp
@@ -22,7 +22,7 @@ template <class Iter>
 std::false_type prev_test(...);
 
 template <class Iter>
-decltype((void) std::prev(std::declval<Iter>()), std::true_type()) prev_test(int);
+decltype((void)std::prev(std::declval<Iter>()), std::true_type()) prev_test(int);
 
 template <class Iter>
 using CanPrev = decltype(prev_test<Iter>(0));
@@ -30,7 +30,7 @@ using CanPrev = decltype(prev_test<Iter>(0));
 static_assert(!CanPrev<cpp17_input_iterator<int*> >::value, "");
 static_assert(CanPrev<bidirectional_iterator<int*> >::value, "");
 #if TEST_STD_VER >= 20
-    static_assert(!CanPrev<cpp20_random_access_iterator<int*> >::value);
+static_assert(!CanPrev<cpp20_random_access_iterator<int*> >::value);
 #endif
 
 template <class It>

>From 23deda335755fc782595e4c41feae65b84ef7ad4 Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Sat, 19 Oct 2024 10:15:35 +0100
Subject: [PATCH 3/4] address code review

---
 libcxx/include/__iterator/prev.h              |  8 ++++----
 .../iterator.operations/prev.verify.cpp       | 19 +++++++++++++++++++
 .../iterator.operations/prev.pass.cpp         | 15 ---------------
 3 files changed, 23 insertions(+), 19 deletions(-)
 create mode 100644 libcxx/test/libcxx/iterators/iterator.primitives/iterator.operations/prev.verify.cpp

diff --git a/libcxx/include/__iterator/prev.h b/libcxx/include/__iterator/prev.h
index d709d0b6787d60..e032f6ac757708 100644
--- a/libcxx/include/__iterator/prev.h
+++ b/libcxx/include/__iterator/prev.h
@@ -39,10 +39,10 @@ prev(_InputIter __x, typename iterator_traits<_InputIter>::difference_type __n)
   return __x;
 }
 
-template <class _BidirectionalIterator,
-          __enable_if_t<__has_bidirectional_iterator_category<_BidirectionalIterator>::value, int> = 0>
-[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX17 _BidirectionalIterator
-prev(_BidirectionalIterator __it) {
+template <class _InputIter, __enable_if_t<__has_input_iterator_category<_InputIter>::value, int> = 0>
+[[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX17 _InputIter prev(_InputIter __it) {
+  static_assert(__has_bidirectional_iterator_category<_InputIter>::value,
+                "Attempt to prev(it) with a non-bidirectional iterator");
   return std::prev(std::move(__it), 1);
 }
 
diff --git a/libcxx/test/libcxx/iterators/iterator.primitives/iterator.operations/prev.verify.cpp b/libcxx/test/libcxx/iterators/iterator.primitives/iterator.operations/prev.verify.cpp
new file mode 100644
index 00000000000000..da0a336815a5c8
--- /dev/null
+++ b/libcxx/test/libcxx/iterators/iterator.primitives/iterator.operations/prev.verify.cpp
@@ -0,0 +1,19 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// std::prev
+
+#include <iterator>
+#include "test_iterators.h"
+
+void test() {
+  int arr[] = {1, 2};
+  cpp17_input_iterator<int*> it(&arr[0]);
+  it = std::prev(it);
+  // expected-error-re@*:* {{static assertion failed due to requirement {{.*}}: Attempt to prev(it) with a non-bidirectional iterator}}
+}
diff --git a/libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp b/libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp
index 97b9aee7045396..784c1b93b7ca89 100644
--- a/libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp
+++ b/libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp
@@ -18,21 +18,6 @@
 #include "test_macros.h"
 #include "test_iterators.h"
 
-template <class Iter>
-std::false_type prev_test(...);
-
-template <class Iter>
-decltype((void)std::prev(std::declval<Iter>()), std::true_type()) prev_test(int);
-
-template <class Iter>
-using CanPrev = decltype(prev_test<Iter>(0));
-
-static_assert(!CanPrev<cpp17_input_iterator<int*> >::value, "");
-static_assert(CanPrev<bidirectional_iterator<int*> >::value, "");
-#if TEST_STD_VER >= 20
-static_assert(!CanPrev<cpp20_random_access_iterator<int*> >::value);
-#endif
-
 template <class It>
 TEST_CONSTEXPR_CXX17 void
 check_prev_n(It it, typename std::iterator_traits<It>::difference_type n, It result)

>From f5289a1422756913ffe4d7daea378cdd268a42b4 Mon Sep 17 00:00:00 2001
From: Hui <hui.xie0621 at gmail.com>
Date: Sat, 19 Oct 2024 10:21:10 +0100
Subject: [PATCH 4/4] add comment

---
 libcxx/include/__iterator/prev.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libcxx/include/__iterator/prev.h b/libcxx/include/__iterator/prev.h
index e032f6ac757708..bffd5527dc953e 100644
--- a/libcxx/include/__iterator/prev.h
+++ b/libcxx/include/__iterator/prev.h
@@ -39,6 +39,9 @@ prev(_InputIter __x, typename iterator_traits<_InputIter>::difference_type __n)
   return __x;
 }
 
+// LWG 3197
+// It is unclear what the implications of "BidirectionalIterator" in the standard are.
+// However, calling std::prev(non-bidi-iterator) is obviously an error and we should catch it at compile time.
 template <class _InputIter, __enable_if_t<__has_input_iterator_category<_InputIter>::value, int> = 0>
 [[__nodiscard__]] _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX17 _InputIter prev(_InputIter __it) {
   static_assert(__has_bidirectional_iterator_category<_InputIter>::value,



More information about the libcxx-commits mailing list