[libc++] Disallow std::prev(non_cpp17_bidi_iterator)

[huixie90]

The std::prev function appeared to work on non Cpp17BidirectionalIterators, however it behaved strangely by being the identity function. That was extremely misleading and potentially dangerous, since several recent iterators that are C++20 bidirectional_iterators don't satisfy the Cpp17BidirectionalIterator named requirement. For example:

auto zv = std::views::zip(vec1, vec2); auto it = zv.begin() + 5; auto it2 = std::prev(it); // "it2" will be the same as "it", instead of the previous one 

Here, zip_view::iterator is a c++20 random_access_iterator, but it only satisfies the Cpp17InputIterator named requirement because its reference type is a proxy type. Hence std::prev would silently accept that iterator but it would do a no-op instead of going to the previous position.

This patch changes std::prev to produce an error at compile-time when instantiated with a type that is not a Cpp17BidirectionalIterator.

Fixes #109456

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Hui (huixie90)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/112102.diff

2 Files Affected:

• (modified) libcxx/include/__iterator/prev.h (+9-1)
• (modified) libcxx/test/std/iterators/iterator.primitives/iterator.operations/prev.pass.cpp (+15)

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) 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.