Skip to content

[libc++][ranges] LWG3692: zip_view::iterator's operator<=> is overconstrained and changes of zip_view in P2165R4 #112077

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libcxx/docs/Status/Cxx23Issues.csv
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@
"`LWG3672 <https://wg21.link/LWG3672>`__","``common_iterator::operator->()`` should return by value","2022-07 (Virtual)","|Complete|","19.0",""
"`LWG3683 <https://wg21.link/LWG3683>`__","``operator==`` for ``polymorphic_allocator`` cannot deduce template argument in common cases","2022-07 (Virtual)","|Complete|","20.0",""
"`LWG3687 <https://wg21.link/LWG3687>`__","``expected<cv void, E>`` move constructor should move","2022-07 (Virtual)","|Complete|","16.0",""
"`LWG3692 <https://wg21.link/LWG3692>`__","``zip_view::iterator``'s ``operator<=>`` is overconstrained","2022-07 (Virtual)","","",""
"`LWG3692 <https://wg21.link/LWG3692>`__","``zip_view::iterator``'s ``operator<=>`` is overconstrained","2022-07 (Virtual)","|Complete|","20.0",""
"`LWG3701 <https://wg21.link/LWG3701>`__","Make ``formatter<remove_cvref_t<const charT[N]>, charT>`` requirement explicit","2022-07 (Virtual)","|Complete|","15.0",""
"`LWG3702 <https://wg21.link/LWG3702>`__","Should ``zip_transform_view::iterator`` remove ``operator<``","2022-07 (Virtual)","","",""
"`LWG3703 <https://wg21.link/LWG3703>`__","Missing requirements for ``expected<T, E>`` requires ``is_void<T>``","2022-07 (Virtual)","|Complete|","16.0",""
Expand Down
2 changes: 1 addition & 1 deletion libcxx/docs/Status/Cxx23Papers.csv
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
"`P1642R11 <https://wg21.link/P1642R11>`__","Freestanding ``[utilities]``, ``[ranges]``, and ``[iterators]``","2022-07 (Virtual)","","",""
"`P1899R3 <https://wg21.link/P1899R3>`__","``stride_view``","2022-07 (Virtual)","","",""
"`P2093R14 <https://wg21.link/P2093R14>`__","Formatted output","2022-07 (Virtual)","|Complete|","18.0",""
"`P2165R4 <https://wg21.link/P2165R4>`__","Compatibility between ``tuple``, ``pair`` and ``tuple-like`` objects","2022-07 (Virtual)","","",""
"`P2165R4 <https://wg21.link/P2165R4>`__","Compatibility between ``tuple``, ``pair`` and ``tuple-like`` objects","2022-07 (Virtual)","|Partial|","","Only the part for ``zip_view`` is implemented."
"`P2278R4 <https://wg21.link/P2278R4>`__","``cbegin`` should always return a constant iterator","2022-07 (Virtual)","","",""
"`P2286R8 <https://wg21.link/P2286R8>`__","Formatting Ranges","2022-07 (Virtual)","|Complete|","16.0",""
"`P2291R3 <https://wg21.link/P2291R3>`__","Add Constexpr Modifiers to Functions ``to_chars`` and ``from_chars`` for Integral Types in ``<charconv>`` Header","2022-07 (Virtual)","|Complete|","16.0",""
Expand Down
55 changes: 8 additions & 47 deletions libcxx/include/__ranges/zip_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
#include <__utility/forward.h>
#include <__utility/integer_sequence.h>
#include <__utility/move.h>
#include <__utility/pair.h>
#include <tuple>

#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
Expand All @@ -58,22 +57,11 @@ concept __zip_is_common =
(!(bidirectional_range<_Ranges> && ...) && (common_range<_Ranges> && ...)) ||
((random_access_range<_Ranges> && ...) && (sized_range<_Ranges> && ...));

template <typename _Tp, typename _Up>
auto __tuple_or_pair_test() -> pair<_Tp, _Up>;

template <typename... _Types>
requires(sizeof...(_Types) != 2)
auto __tuple_or_pair_test() -> tuple<_Types...>;

template <class... _Types>
using __tuple_or_pair = decltype(__tuple_or_pair_test<_Types...>());

template <class _Fun, class _Tuple>
_LIBCPP_HIDE_FROM_ABI constexpr auto __tuple_transform(_Fun&& __f, _Tuple&& __tuple) {
return std::apply(
[&]<class... _Types>(_Types&&... __elements) {
return __tuple_or_pair<invoke_result_t<_Fun&, _Types>...>(
std::invoke(__f, std::forward<_Types>(__elements))...);
return tuple<invoke_result_t<_Fun&, _Types>...>(std::invoke(__f, std::forward<_Types>(__elements))...);
},
std::forward<_Tuple>(__tuple));
}
Expand All @@ -88,7 +76,7 @@ _LIBCPP_HIDE_FROM_ABI constexpr void __tuple_for_each(_Fun&& __f, _Tuple&& __tup
}

template <class _Fun, class _Tuple1, class _Tuple2, size_t... _Indices>
_LIBCPP_HIDE_FROM_ABI constexpr __tuple_or_pair<
_LIBCPP_HIDE_FROM_ABI constexpr tuple<
invoke_result_t<_Fun&,
typename tuple_element<_Indices, remove_cvref_t<_Tuple1>>::type,
typename tuple_element<_Indices, remove_cvref_t<_Tuple2>>::type>...>
Expand Down Expand Up @@ -250,10 +238,9 @@ template <input_range... _Views>
requires(view<_Views> && ...) && (sizeof...(_Views) > 0)
template <bool _Const>
class zip_view<_Views...>::__iterator : public __zip_view_iterator_category_base<_Const, _Views...> {
__tuple_or_pair<iterator_t<__maybe_const<_Const, _Views>>...> __current_;
tuple<iterator_t<__maybe_const<_Const, _Views>>...> __current_;

_LIBCPP_HIDE_FROM_ABI constexpr explicit __iterator(
__tuple_or_pair<iterator_t<__maybe_const<_Const, _Views>>...> __current)
_LIBCPP_HIDE_FROM_ABI constexpr explicit __iterator(tuple<iterator_t<__maybe_const<_Const, _Views>>...> __current)
: __current_(std::move(__current)) {}

template <bool>
Expand All @@ -266,7 +253,7 @@ class zip_view<_Views...>::__iterator : public __zip_view_iterator_category_base

public:
using iterator_concept = decltype(__get_zip_view_iterator_tag<_Const, _Views...>());
using value_type = __tuple_or_pair<range_value_t<__maybe_const<_Const, _Views>>...>;
using value_type = tuple<range_value_t<__maybe_const<_Const, _Views>>...>;
using difference_type = common_type_t<range_difference_t<__maybe_const<_Const, _Views>>...>;

_LIBCPP_HIDE_FROM_ABI __iterator() = default;
Expand Down Expand Up @@ -340,33 +327,8 @@ class zip_view<_Views...>::__iterator : public __zip_view_iterator_category_base
}
}

_LIBCPP_HIDE_FROM_ABI friend constexpr bool operator<(const __iterator& __x, const __iterator& __y)
requires __zip_all_random_access<_Const, _Views...>
{
return __x.__current_ < __y.__current_;
}

_LIBCPP_HIDE_FROM_ABI friend constexpr bool operator>(const __iterator& __x, const __iterator& __y)
requires __zip_all_random_access<_Const, _Views...>
{
return __y < __x;
}

_LIBCPP_HIDE_FROM_ABI friend constexpr bool operator<=(const __iterator& __x, const __iterator& __y)
requires __zip_all_random_access<_Const, _Views...>
{
return !(__y < __x);
}

_LIBCPP_HIDE_FROM_ABI friend constexpr bool operator>=(const __iterator& __x, const __iterator& __y)
requires __zip_all_random_access<_Const, _Views...>
{
return !(__x < __y);
}

_LIBCPP_HIDE_FROM_ABI friend constexpr auto operator<=>(const __iterator& __x, const __iterator& __y)
requires __zip_all_random_access<_Const, _Views...> &&
(three_way_comparable<iterator_t<__maybe_const<_Const, _Views>>> && ...)
requires __zip_all_random_access<_Const, _Views...>
{
return __x.__current_ <=> __y.__current_;
}
Expand Down Expand Up @@ -427,10 +389,9 @@ template <input_range... _Views>
requires(view<_Views> && ...) && (sizeof...(_Views) > 0)
template <bool _Const>
class zip_view<_Views...>::__sentinel {
__tuple_or_pair<sentinel_t<__maybe_const<_Const, _Views>>...> __end_;
tuple<sentinel_t<__maybe_const<_Const, _Views>>...> __end_;

_LIBCPP_HIDE_FROM_ABI constexpr explicit __sentinel(
__tuple_or_pair<sentinel_t<__maybe_const<_Const, _Views>>...> __end)
_LIBCPP_HIDE_FROM_ABI constexpr explicit __sentinel(tuple<sentinel_t<__maybe_const<_Const, _Views>>...> __end)
: __end_(__end) {}

friend class zip_view<_Views...>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,7 @@ constexpr bool test() {
std::ranges::zip_view<std::ranges::zip_view<SizedRandomAccessView, SizedRandomAccessView>>> decltype(auto) v2 =
std::views::zip(v);

#ifdef _LIBCPP_VERSION // libc++ doesn't implement P2165R4 yet
static_assert(std::is_same_v<std::ranges::range_reference_t<decltype(v2)>, std::tuple<std::pair<int&, int&>>>);
#else
static_assert(std::is_same_v<std::ranges::range_reference_t<decltype(v2)>, std::tuple<std::tuple<int&, int&>>>);
#endif
}
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,8 @@ constexpr bool test() {
using View = std::ranges::zip_view<DefaultConstructibleView, DefaultConstructibleView>;
View v = View(); // the default constructor is not explicit
assert(v.size() == 3);
auto it = v.begin();
#ifdef _LIBCPP_VERSION // libc++ doesn't implement P2165R4 yet
using Value = std::pair<const int&, const int&>;
#else
auto it = v.begin();
using Value = std::tuple<const int&, const int&>;
#endif
assert(*it++ == Value(buff[0], buff[0]));
assert(*it++ == Value(buff[1], buff[1]));
assert(*it == Value(buff[2], buff[2]));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,8 @@

// friend constexpr bool operator==(const iterator& x, const iterator& y)
// requires (equality_comparable<iterator_t<maybe-const<Const, Views>>> && ...);
// friend constexpr bool operator<(const iterator& x, const iterator& y)
// requires all-random-access<Const, Views...>;
// friend constexpr bool operator>(const iterator& x, const iterator& y)
// requires all-random-access<Const, Views...>;
// friend constexpr bool operator<=(const iterator& x, const iterator& y)
// requires all-random-access<Const, Views...>;
// friend constexpr bool operator>=(const iterator& x, const iterator& y)
// requires all-random-access<Const, Views...>;
// friend constexpr auto operator<=>(const iterator& x, const iterator& y)
// requires all-random-access<Const, Views...> &&
// (three_way_comparable<iterator_t<maybe-const<Const, Views>>> && ...);
// requires all-random-access<Const, Views...>;

#include <ranges>
#include <compare>
Expand Down Expand Up @@ -165,12 +156,7 @@ constexpr bool test() {
using Subrange = std::ranges::subrange<It>;
static_assert(!std::three_way_comparable<It>);
using R = std::ranges::zip_view<Subrange, Subrange>;
#ifdef _LIBCPP_VERSION
// libc++ hasn't implemented LWG-3692 "zip_view::iterator's operator<=> is overconstrained"
static_assert(!std::three_way_comparable<std::ranges::iterator_t<R>>);
#else
static_assert(std::three_way_comparable<std::ranges::iterator_t<R>>);
#endif

int a[] = {1, 2, 3, 4};
int b[] = {5, 6, 7, 8, 9};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,7 @@ constexpr bool test() {
auto [x, y] = *it;
assert(&x == &(a[0]));
assert(&y == &(b[0]));
#ifdef _LIBCPP_VERSION // libc++ doesn't implement P2165R4 yet
static_assert(std::is_same_v<decltype(*it), std::pair<int&, double&>>);
#else
static_assert(std::is_same_v<decltype(*it), std::tuple<int&, double&>>);
#endif

x = 5;
y = 0.1;
Expand All @@ -70,11 +66,7 @@ constexpr bool test() {
auto it = v.begin();
assert(&(std::get<0>(*it)) == &(a[0]));
assert(&(std::get<1>(*it)) == &(a[0]));
#ifdef _LIBCPP_VERSION // libc++ doesn't implement P2165R4 yet
static_assert(std::is_same_v<decltype(*it), std::pair<int&, int const&>>);
#else
static_assert(std::is_same_v<decltype(*it), std::tuple<int&, int const&>>);
#endif
}
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,15 @@ struct ConstVeryDifferentRange {
void test() {
int buffer[] = {1, 2, 3, 4};
{
// 2 views should have pair value_type
// 2 views should have 2-tuple value_type
// random_access_iterator_tag
std::ranges::zip_view v(buffer, buffer);
using Iter = decltype(v.begin());

static_assert(std::is_same_v<Iter::iterator_concept, std::random_access_iterator_tag>);
static_assert(std::is_same_v<Iter::iterator_category, std::input_iterator_tag>);
static_assert(std::is_same_v<Iter::difference_type, std::ptrdiff_t>);
#ifdef _LIBCPP_VERSION // libc++ doesn't implement P2165R4 yet
static_assert(std::is_same_v<Iter::value_type, std::pair<int, int>>);
#else
static_assert(std::is_same_v<Iter::value_type, std::tuple<int, int>>);
#endif
static_assert(HasIterCategory<Iter>);
}

Expand Down Expand Up @@ -124,11 +120,7 @@ void test() {
static_assert(std::is_same_v<Iter::iterator_concept, std::random_access_iterator_tag>);
static_assert(std::is_same_v<Iter::iterator_category, std::input_iterator_tag>);
static_assert(std::is_same_v<Iter::difference_type, std::ptrdiff_t>);
#ifdef _LIBCPP_VERSION // libc++ doesn't implement P2165R4 yet
static_assert(std::is_same_v<Iter::value_type, std::pair<int, std::pair<int, int>>>);
#else
static_assert(std::is_same_v<Iter::value_type, std::tuple<int, std::tuple<int, int>>>);
#endif
static_assert(HasIterCategory<Iter>);
}

Expand Down Expand Up @@ -169,11 +161,7 @@ void test() {
// value_type of multiple views with different value_type
std::ranges::zip_view v{foos, bars};
using Iter = decltype(v.begin());
#ifdef _LIBCPP_VERSION // libc++ doesn't implement P2165R4 yet
static_assert(std::is_same_v<Iter::value_type, std::pair<Foo, Bar>>);
#else
static_assert(std::is_same_v<Iter::value_type, std::tuple<Foo, Bar>>);
#endif
}

{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,7 @@ constexpr bool test() {
assert(it[2] == *(it + 2));
assert(it[4] == *(it + 4));

#ifdef _LIBCPP_VERSION // libc++ doesn't implement P2165R4 yet
static_assert(std::is_same_v<decltype(it[2]), std::pair<int&, int>>);
#else
static_assert(std::is_same_v<decltype(it[2]), std::tuple<int&, int>>);
#endif
}

{
Expand All @@ -42,11 +38,7 @@ constexpr bool test() {
assert(it[2] == *(it + 2));
assert(it[4] == *(it + 4));

#ifdef _LIBCPP_VERSION // libc++ doesn't implement P2165R4 yet
static_assert(std::is_same_v<decltype(it[2]), std::pair<int&, int&>>);
#else
static_assert(std::is_same_v<decltype(it[2]), std::tuple<int&, int&>>);
#endif
}

{
Expand Down
Loading