libstdc++: Teach std::distance and std::advance about C++20 iterators [PR102181]
When the C++98 std::distance and std::advance functions (and C++11 std::next and std::prev) are used with C++20 iterators there can be unexpected results, ranging from compilation failure to decreased performance to undefined behaviour. An iterator which satisfies std::input_iterator but does not meet the Cpp17InputIterator requirements might have std::output_iterator_tag for its std::iterator_traits<I>::iterator_category, which means it currently cannot be used with std::advance at all. However, the implementation of std::advance for a Cpp17InputIterator doesn't do anything that isn't valid for iterator types satsifying C++20 std::input_iterator. Similarly, a type satisfying C++20 std::bidirectional_iterator might be usable with std::prev, if it weren't for the fact that its C++17 iterator_category is std::input_iterator_tag. Finally, a type satisfying C++20 std::random_access_iterator might use a slower implementation for std::distance or std::advance if its C++17 iterator_category is not std::random_access_iterator_tag. This commit adds a __promotable_iterator concept to detect C++20 iterators which explicitly define an iterator_concept member, and which either have no iterator_category, or their iterator_category is weaker than their iterator_concept. This is used by std::distance and std::advance to detect iterators which should dispatch based on their iterator_concept instead of their iterator_category. This means that those functions just work and do the right thing for C++20 iterators which would otherwise fail to compile or have suboptimal performance. This is related to LWG 3197, which considers making it undefined to use std::prev with types which do not meet the Cpp17BidirectionalIterator requirements. I think making it work, as in this commit, is a better solution than banning it (or rejecting it at compile-time as libc++ does). PR libstdc++/102181 libstdc++-v3/ChangeLog: * include/bits/stl_iterator_base_funcs.h (distance, advance): Check C++20 iterator concepts and handle appropriately. (__detail::__iter_category_converts_to_concept): New concept. (__detail::__promotable_iterator): New concept. * testsuite/24_iterators/operations/cxx20_iterators.cc: New test. Reviewed-by: Patrick Palka <ppalka@redhat.com> Reviewed-by: Tomasz Kamiński <tkaminsk@redhat.com>
This commit is contained in:
committed by
Tomasz Kamiński
parent
95f517dc77
commit
86dc3b61c3
@@ -130,6 +130,28 @@ _GLIBCXX_END_NAMESPACE_CONTAINER
|
||||
__distance(_OutputIterator, _OutputIterator, output_iterator_tag) = delete;
|
||||
#endif
|
||||
|
||||
#ifdef __glibcxx_concepts
|
||||
namespace __detail
|
||||
{
|
||||
// Satisfied if ITER_TRAITS(Iter)::iterator_category is valid and is
|
||||
// at least as strong as ITER_TRAITS(Iter)::iterator_concept.
|
||||
template<typename _Iter>
|
||||
concept __iter_category_converts_to_concept
|
||||
= convertible_to<typename __iter_traits<_Iter>::iterator_category,
|
||||
typename __iter_traits<_Iter>::iterator_concept>;
|
||||
|
||||
// Satisfied if the type is a C++20 iterator that defines iterator_concept,
|
||||
// and its iterator_concept is stronger than its iterator_category (if any).
|
||||
// Used by std::distance and std::advance to detect iterators which should
|
||||
// dispatch based on their C++20 concept not their C++17 category.
|
||||
template<typename _Iter>
|
||||
concept __promotable_iterator
|
||||
= input_iterator<_Iter>
|
||||
&& requires { typename __iter_traits<_Iter>::iterator_concept; }
|
||||
&& ! __iter_category_converts_to_concept<_Iter>;
|
||||
} // namespace __detail
|
||||
#endif
|
||||
|
||||
/**
|
||||
* @brief A generalization of pointer arithmetic.
|
||||
* @param __first An input iterator.
|
||||
@@ -149,6 +171,24 @@ _GLIBCXX_END_NAMESPACE_CONTAINER
|
||||
typename iterator_traits<_InputIterator>::difference_type
|
||||
distance(_InputIterator __first, _InputIterator __last)
|
||||
{
|
||||
#ifdef __glibcxx_concepts
|
||||
// A type which satisfies the C++20 random_access_iterator concept might
|
||||
// have input_iterator_tag as its iterator_category type, which would
|
||||
// mean we select the O(n) __distance. Or a C++20 std::input_iterator
|
||||
// that is not a Cpp17InputIterator might have output_iterator_tag as
|
||||
// its iterator_category type and then calling __distance with
|
||||
// std::__iterator_category(__first) would be ill-formed.
|
||||
// So for C++20 iterator types we can just choose to do the right thing.
|
||||
if constexpr (__detail::__promotable_iterator<_InputIterator>)
|
||||
{
|
||||
if constexpr (random_access_iterator<_InputIterator>)
|
||||
return __last - __first;
|
||||
else
|
||||
return std::__distance(std::move(__first), std::move(__last),
|
||||
input_iterator_tag());
|
||||
}
|
||||
else // assume it meets the Cpp17InputIterator requirements:
|
||||
#endif
|
||||
// concept requirements -- taken care of in __distance
|
||||
return std::__distance(__first, __last,
|
||||
std::__iterator_category(__first));
|
||||
@@ -221,9 +261,31 @@ _GLIBCXX_END_NAMESPACE_CONTAINER
|
||||
inline _GLIBCXX17_CONSTEXPR void
|
||||
advance(_InputIterator& __i, _Distance __n)
|
||||
{
|
||||
// concept requirements -- taken care of in __advance
|
||||
typename iterator_traits<_InputIterator>::difference_type __d = __n;
|
||||
std::__advance(__i, __d, std::__iterator_category(__i));
|
||||
#ifdef __glibcxx_concepts
|
||||
// A type which satisfies the C++20 bidirectional_iterator concept might
|
||||
// have input_iterator_tag as its iterator_category type, which would
|
||||
// mean we select the __advance overload which cannot move backwards.
|
||||
// A C++20 random_access_iterator we might select the O(n) __advance
|
||||
// if it doesn't meet the Cpp17RandomAccessIterator requirements.
|
||||
// So for C++20 iterator types we can just choose to do the right thing.
|
||||
if constexpr (__detail::__promotable_iterator<_InputIterator>
|
||||
&& ranges::__detail::__is_integer_like<_Distance>)
|
||||
{
|
||||
auto __d = static_cast<iter_difference_t<_InputIterator>>(__n);
|
||||
if constexpr (random_access_iterator<_InputIterator>)
|
||||
std::__advance(__i, __d, random_access_iterator_tag());
|
||||
else if constexpr (bidirectional_iterator<_InputIterator>)
|
||||
std::__advance(__i, __d, bidirectional_iterator_tag());
|
||||
else
|
||||
std::__advance(__i, __d, input_iterator_tag());
|
||||
}
|
||||
else // assume it meets the Cpp17InputIterator requirements:
|
||||
#endif
|
||||
{
|
||||
// concept requirements -- taken care of in __advance
|
||||
typename iterator_traits<_InputIterator>::difference_type __d = __n;
|
||||
std::__advance(__i, __d, std::__iterator_category(__i));
|
||||
}
|
||||
}
|
||||
|
||||
#if __cplusplus >= 201103L
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
// { dg-do run { target c++20 } }
|
||||
|
||||
#include <ranges>
|
||||
#include <testsuite_iterators.h>
|
||||
#include <testsuite_hooks.h>
|
||||
|
||||
// Bug 102181 std::advance and std::views::iota<std::int64_t> don't work
|
||||
void
|
||||
test_pr102181()
|
||||
{
|
||||
#ifdef __SIZEOF_INT128__
|
||||
using type = unsigned __int128;
|
||||
#else
|
||||
using type = unsigned long;
|
||||
#endif
|
||||
auto v = std::ranges::iota_view(type(0), type(10));
|
||||
auto b = v.begin();
|
||||
VERIFY( std::distance(b, std::next(b)) == 1 );
|
||||
std::advance(b, std::iter_difference_t<decltype(b)>(1));
|
||||
VERIFY( *b == 1 );
|
||||
VERIFY( std::distance(b, v.end()) == 9 );
|
||||
}
|
||||
|
||||
// https://stackoverflow.com/questions/68100775/rangesviewtransform-produces-an-inputiterator-preventing-the-use-of-stdpre
|
||||
void
|
||||
test_transform_view_iterator()
|
||||
{
|
||||
int a[] = {0, 1, 2, 3};
|
||||
__gnu_test::random_access_container<int> rr(a);
|
||||
auto rx = std::ranges::views::transform(rr, std::identity{});
|
||||
auto re = rx.end();
|
||||
VERIFY( *std::prev(re) == 3 );
|
||||
VERIFY( std::distance(rx.begin(), re) == 4 );
|
||||
|
||||
__gnu_test::bidirectional_container<int> br(a);
|
||||
auto bx = std::ranges::views::transform(br, std::identity{});
|
||||
auto be = bx.end();
|
||||
VERIFY( *std::prev(be) == 3 );
|
||||
VERIFY( std::distance(bx.begin(), be) == 4 );
|
||||
|
||||
__gnu_test::forward_container<int> fr(a);
|
||||
auto fx = std::ranges::views::transform(br, std::identity{});
|
||||
auto fb = fx.begin();
|
||||
VERIFY( *std::next(fb) == 1 );
|
||||
VERIFY( std::distance(fb, fx.end()) == 4 );
|
||||
|
||||
__gnu_test::test_input_range<int> ir(a);
|
||||
auto ix = std::ranges::views::transform(ir, std::identity{});
|
||||
auto ii = ix.begin();
|
||||
std::advance(ii, 1);
|
||||
VERIFY( *ii == 1 );
|
||||
// N.B. cannot use std::distance or std::next here because there is no
|
||||
// iterator_traits<decltype(ii)>::difference_type for this iterator.
|
||||
}
|
||||
|
||||
int main()
|
||||
{
|
||||
test_pr102181();
|
||||
test_transform_view_iterator();
|
||||
}
|
||||
Reference in New Issue
Block a user