[libc++][ranges] LWG3692: zip_view::iterator's operator<=> is overconstrained and changes of zip_view in P2165R4
#112077
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: A. Jiang (frederick-vs-ja) ChangesThe changes are nearly pure simplifications, so I think it's OK to do them together in the same PR. Actual test coverages were already added in commit ad41d1e (https://reviews.llvm.org/D141216). Thanks to @CaseyCarter! Fixes #104975. Towards #105200. Full diff: https://github.com/llvm/llvm-project/pull/112077.diff 9 Files Affected:
diff --git a/libcxx/docs/Status/Cxx23Issues.csv b/libcxx/docs/Status/Cxx23Issues.csv
index 63e4176ecba1d7..cfa721230e5fb8 100644
--- a/libcxx/docs/Status/Cxx23Issues.csv
+++ b/libcxx/docs/Status/Cxx23Issues.csv
@@ -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",""
diff --git a/libcxx/docs/Status/Cxx23Papers.csv b/libcxx/docs/Status/Cxx23Papers.csv
index da7b5881877135..80636f22dacc76 100644
--- a/libcxx/docs/Status/Cxx23Papers.csv
+++ b/libcxx/docs/Status/Cxx23Papers.csv
@@ -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",""
diff --git a/libcxx/include/__ranges/zip_view.h b/libcxx/include/__ranges/zip_view.h
index fe3c87a9306fe9..3ff49b8c1c6267 100644
--- a/libcxx/include/__ranges/zip_view.h
+++ b/libcxx/include/__ranges/zip_view.h
@@ -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)
@@ -58,21 +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>...>(
+ return tuple<invoke_result_t<_Fun&, _Types>...>(
std::invoke(__f, std::forward<_Types>(__elements))...);
},
std::forward<_Tuple>(__tuple));
@@ -88,7 +77,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>...>
@@ -250,10 +239,10 @@ 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)
+ tuple<iterator_t<__maybe_const<_Const, _Views>>...> __current)
: __current_(std::move(__current)) {}
template <bool>
@@ -266,7 +255,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;
@@ -340,33 +329,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_;
}
@@ -427,10 +391,10 @@ 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)
+ tuple<sentinel_t<__maybe_const<_Const, _Views>>...> __end)
: __end_(__end) {}
friend class zip_view<_Views...>;
diff --git a/libcxx/test/std/ranges/range.adaptors/range.zip/cpo.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.zip/cpo.pass.cpp
index ea5953cefa0ff3..bdfd58ff8bbe78 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.zip/cpo.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.zip/cpo.pass.cpp
@@ -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;
}
diff --git a/libcxx/test/std/ranges/range.adaptors/range.zip/ctor.default.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.zip/ctor.default.pass.cpp
index f53289621eabba..fdfcc02a8fb1c1 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.zip/ctor.default.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.zip/ctor.default.pass.cpp
@@ -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]));
diff --git a/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/compare.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/compare.pass.cpp
index ed1cb0ccebd2b5..8ab7346800093e 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/compare.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/compare.pass.cpp
@@ -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>
@@ -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};
diff --git a/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/deref.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/deref.pass.cpp
index 569d0409721941..fb58aa28fbdf8d 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/deref.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/deref.pass.cpp
@@ -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;
@@ -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;
}
diff --git a/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/member_types.compile.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/member_types.compile.pass.cpp
index c19f6c2b16524f..2f2f0fc4f4e331 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/member_types.compile.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/member_types.compile.pass.cpp
@@ -65,7 +65,7 @@ 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());
@@ -73,11 +73,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, int>>);
-#else
static_assert(std::is_same_v<Iter::value_type, std::tuple<int, int>>);
-#endif
static_assert(HasIterCategory<Iter>);
}
@@ -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>);
}
@@ -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
}
{
diff --git a/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/subscript.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/subscript.pass.cpp
index 1538d763205db1..ba3abfa2a43695 100644
--- a/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/subscript.pass.cpp
+++ b/libcxx/test/std/ranges/range.adaptors/range.zip/iterator/subscript.pass.cpp
@@ -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
}
{
@@ -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
}
{
|
…rconstrained and changes of `zip_view` in P2165R4 The changes are nearly pure simplifications, so I think it's OK to do them together in the same PR. Actual test coverages were already added in commit ad41d1e (https://reviews.llvm.org/D141216). Thanks to @CaseyCarter!
frederick-vs-ja
force-pushed
the
lwg-3692-p2165r4-zip_view
branch
from
33f3a61
to
abc7b30
Compare
CaseyCarter
approved these changes
Oct 12, 2024
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
bricknerb
pushed a commit
to bricknerb/llvm-project
that referenced
this pull request
Oct 17, 2024
…rconstrained and changes of `zip_view` in P2165R4 (llvm#112077) The changes are nearly pure simplifications, so I think it's OK to do them together in the same PR. Actual test coverages were already added in commit ad41d1e (https://reviews.llvm.org/D141216). Thanks to Casey Carter! Fixes llvm#104975 Towards llvm#105200
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The changes are nearly pure simplifications, so I think it's OK to do them together in the same PR.
Actual test coverages were already added in commit ad41d1e (https://reviews.llvm.org/D141216). Thanks to @CaseyCarter!
Fixes #104975. Towards #105200.