Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[libc++][spaceship] Implements X::iterator container requirements. #99343

Merged
merged 6 commits into from
Jul 24, 2024
Merged

[libc++][spaceship] Implements X::iterator container requirements. #99343

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fcc71b5
[libc++][spaceship] Implements X::iterator container requirements.
mordante Jul 17, 2024
2dc8392
bounded_iterator
mordante Jul 20, 2024
cd9e497
CI fixes.
mordante Jul 20, 2024
0739782
CI fixes and improvements
mordante Jul 20, 2024
ecd6887
Polish the patch.
mordante Jul 21, 2024
e94dc78
Addresses review comments.
mordante Jul 23, 2024
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/Cxx20Issues.csv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@
"`3349 <https://wg21.link/LWG3349>`__","Missing ``__cpp_lib_constexpr_complex``\ for P0415R1","Prague","|Complete|","16.0"
"`3350 <https://wg21.link/LWG3350>`__","Simplify return type of ``lexicographical_compare_three_way``\ ","Prague","|Complete|","17.0","|spaceship|"
"`3351 <https://wg21.link/LWG3351>`__","``ranges::enable_safe_range``\ should not be constrained","Prague","|Complete|","15.0","|ranges|"
"`3352 <https://wg21.link/LWG3352>`__","``strong_equality``\ isn't a thing","Prague","|Nothing To Do|","","|spaceship|"
"`3352 <https://wg21.link/LWG3352>`__","``strong_equality``\ isn't a thing","Prague","|Complete|","19.0","|spaceship|"
"`3354 <https://wg21.link/LWG3354>`__","``has_strong_structural_equality``\ has a meaningless definition","Prague","|Nothing To Do|","","|spaceship|"
"`3355 <https://wg21.link/LWG3355>`__","The memory algorithms should support move-only input iterators introduced by P1207","Prague","|Complete|","15.0","|ranges|"
"`3356 <https://wg21.link/LWG3356>`__","``__cpp_lib_nothrow_convertible``\ should be ``__cpp_lib_is_nothrow_convertible``\ ","Prague","|Complete|","12.0"
Expand Down
2 changes: 1 addition & 1 deletion libcxx/docs/Status/SpaceshipProjects.csv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ Section,Description,Dependencies,Assignee,Complete
"| `[string.view.synop] <https://wg21.link/string.view.synop>`_
| `[string.view.comparison] <https://wg21.link/string.view.comparison>`_",| `basic_string_view <https://reviews.llvm.org/D130295>`_,None,Mark de Wever,|Complete|
- `5.7 Clause 22: Containers library <https://wg21.link/p1614r2#clause-22-containers-library>`_,,,,
| `[container.requirements.general] <https://wg21.link/container.requirements.general>`_,|,None,Unassigned,|Not Started|
| `[container.requirements.general] <https://wg21.link/container.requirements.general>`_,|,None,Mark de Wever,|Complete|
mordante marked this conversation as resolved.
Show resolved Hide resolved
| `[array.syn] <https://wg21.link/array.syn>`_ (`general <https://wg21.link/container.opt.reqmts>`_),| `array <https://reviews.llvm.org/D132265>`_,[expos.only.func],"| Adrian Vogelsgesang
| Hristo Hristov",|Complete|
| `[deque.syn] <https://wg21.link/deque.syn>`_ (`general <https://wg21.link/container.opt.reqmts>`_),| `deque <https://reviews.llvm.org/D144821>`_,[expos.only.func],Hristo Hristov,|Complete|
Expand Down
14 changes: 14 additions & 0 deletions libcxx/include/__bit_reference
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <__bit/countr.h>
#include <__bit/invert_if.h>
#include <__bit/popcount.h>
#include <__compare/ordering.h>
#include <__config>
#include <__fwd/bit_reference.h>
#include <__iterator/iterator_traits.h>
Expand Down Expand Up @@ -913,6 +914,7 @@ public:
return __x.__seg_ == __y.__seg_ && __x.__ctz_ == __y.__ctz_;
}

#if _LIBCPP_STD_VER <= 17
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 friend bool
operator!=(const __bit_iterator& __x, const __bit_iterator& __y) {
return !(__x == __y);
Expand All @@ -937,6 +939,18 @@ public:
operator>=(const __bit_iterator& __x, const __bit_iterator& __y) {
return !(__x < __y);
}
#else // _LIBCPP_STD_VER <= 17
_LIBCPP_HIDE_FROM_ABI constexpr friend strong_ordering
operator<=>(const __bit_iterator& __x, const __bit_iterator& __y) {
if (__x.__seg_ < __y.__seg_)
return strong_ordering::less;

if (__x.__seg_ == __y.__seg_)
return __x.__ctz_ <=> __y.__ctz_;

return strong_ordering::greater;
}
#endif // _LIBCPP_STD_VER <= 17

private:
_LIBCPP_HIDE_FROM_ABI
Expand Down
23 changes: 23 additions & 0 deletions libcxx/include/__iterator/bounded_iter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
#define _LIBCPP___ITERATOR_BOUNDED_ITER_H

#include <__assert>
#include <__compare/ordering.h>
#include <__compare/three_way_comparable.h>
#include <__config>
#include <__iterator/iterator_traits.h>
#include <__memory/pointer_traits.h>
Expand Down Expand Up @@ -201,10 +203,14 @@ struct __bounded_iter {
operator==(__bounded_iter const& __x, __bounded_iter const& __y) _NOEXCEPT {
return __x.__current_ == __y.__current_;
}

#if _LIBCPP_STD_VER <= 17
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR friend bool
operator!=(__bounded_iter const& __x, __bounded_iter const& __y) _NOEXCEPT {
return __x.__current_ != __y.__current_;
}
#endif

_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR friend bool
mordante marked this conversation as resolved.
Show resolved Hide resolved
operator<(__bounded_iter const& __x, __bounded_iter const& __y) _NOEXCEPT {
return __x.__current_ < __y.__current_;
Expand All @@ -222,6 +228,23 @@ struct __bounded_iter {
return __x.__current_ >= __y.__current_;
}

#if _LIBCPP_STD_VER >= 20
_LIBCPP_HIDE_FROM_ABI constexpr friend strong_ordering
operator<=>(__bounded_iter const& __x, __bounded_iter const& __y) noexcept {
if constexpr (three_way_comparable<_Iterator, strong_ordering>) {
return __x.__current_ <=> __y.__current_;
} else {
if (__x.__current_ < __y.__current_)
return strong_ordering::less;

if (__x.__current_ == __y.__current_)
return strong_ordering::equal;

return strong_ordering::greater;
}
}
#endif // _LIBCPP_STD_VER >= 20

private:
template <class>
friend struct pointer_traits;
Expand Down
40 changes: 40 additions & 0 deletions libcxx/include/__iterator/wrap_iter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#ifndef _LIBCPP___ITERATOR_WRAP_ITER_H
#define _LIBCPP___ITERATOR_WRAP_ITER_H

#include <__compare/ordering.h>
mordante marked this conversation as resolved.
Show resolved Hide resolved
#include <__compare/three_way_comparable.h>
#include <__config>
#include <__iterator/iterator_traits.h>
#include <__memory/addressof.h>
Expand Down Expand Up @@ -131,6 +133,7 @@ operator<(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXC
return __x.base() < __y.base();
}

#if _LIBCPP_STD_VER <= 17
template <class _Iter1>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR bool
operator!=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter1>& __y) _NOEXCEPT {
Expand All @@ -142,6 +145,7 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR bool
operator!=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT {
return !(__x == __y);
}
#endif

template <class _Iter1>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR bool
Expand Down Expand Up @@ -179,6 +183,42 @@ operator<=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEX
return !(__y < __x);
}

#if _LIBCPP_STD_VER >= 20

template <class _Iter1>
_LIBCPP_HIDE_FROM_ABI constexpr strong_ordering
operator<=>(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter1>& __y) noexcept {
if constexpr (three_way_comparable<_Iter1, strong_ordering>) {
return __x.base() <=> __y.base();
} else {
if (__x.base() < __y.base())
return strong_ordering::less;

if (__x.base() == __y.base())
return strong_ordering::equal;

return strong_ordering::greater;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we simply get rid of this entirely? I think the reason we have those for operator< & friends was because we ran into some issue, but I am not certain that issue also exists for operator<=>. I would try removing it and see if that works.


template <class _Iter1, class _Iter2>
_LIBCPP_HIDE_FROM_ABI constexpr strong_ordering
operator<=>(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) noexcept {
if constexpr (three_way_comparable_with<_Iter1, _Iter2, strong_ordering>) {
return __x.base() <=> __y.base();
} else {
if (__x.base() < __y.base())
return strong_ordering::less;

if (__x.base() == __y.base())
return strong_ordering::equal;

return strong_ordering::greater;
}
}

#endif // _LIBCPP_STD_VER >= 20

template <class _Iter1, class _Iter2>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14
#ifndef _LIBCPP_CXX03_LANG
Expand Down
29 changes: 27 additions & 2 deletions libcxx/include/deque
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,9 +376,11 @@ public:
return __x.__ptr_ == __y.__ptr_;
}

#if _LIBCPP_STD_VER <= 17
_LIBCPP_HIDE_FROM_ABI friend bool operator!=(const __deque_iterator& __x, const __deque_iterator& __y) {
return !(__x == __y);
}
#endif

_LIBCPP_HIDE_FROM_ABI friend bool operator<(const __deque_iterator& __x, const __deque_iterator& __y) {
return __x.__m_iter_ < __y.__m_iter_ || (__x.__m_iter_ == __y.__m_iter_ && __x.__ptr_ < __y.__ptr_);
Expand All @@ -396,6 +398,30 @@ public:
return !(__x < __y);
}

#if _LIBCPP_STD_VER >= 20
// template <class _Tp = void>
mordante marked this conversation as resolved.
Show resolved Hide resolved
_LIBCPP_HIDE_FROM_ABI friend strong_ordering operator<=>(const __deque_iterator& __x, const __deque_iterator& __y) {
if (__x.__m_iter_ < __y.__m_iter_)
return strong_ordering::less;

if (__x.__m_iter_ == __y.__m_iter_) {
if constexpr (three_way_comparable<pointer, strong_ordering>) {
return __x.__ptr_ <=> __y.__ptr_;
} else {
if (__x.__ptr_ < __y.__ptr_)
return strong_ordering::less;

if (__x.__ptr_ == __y.__ptr_)
return strong_ordering::equal;

return strong_ordering::greater;
}
}

return strong_ordering::greater;
}
#endif // _LIBCPP_STD_VER >= 20

private:
_LIBCPP_HIDE_FROM_ABI explicit __deque_iterator(__map_iterator __m, pointer __p) _NOEXCEPT
: __m_iter_(__m),
Expand Down Expand Up @@ -2530,8 +2556,7 @@ inline _LIBCPP_HIDE_FROM_ABI bool operator<=(const deque<_Tp, _Allocator>& __x,
template <class _Tp, class _Allocator>
_LIBCPP_HIDE_FROM_ABI __synth_three_way_result<_Tp>
operator<=>(const deque<_Tp, _Allocator>& __x, const deque<_Tp, _Allocator>& __y) {
return std::lexicographical_compare_three_way(
__x.begin(), __x.end(), __y.begin(), __y.end(), std::__synth_three_way);
return std::lexicographical_compare_three_way(__x.begin(), __x.end(), __y.begin(), __y.end(), std::__synth_three_way);
}

#endif // _LIBCPP_STD_VER <= 17
Expand Down
18 changes: 16 additions & 2 deletions libcxx/test/libcxx/iterators/bounded_iter/comparison.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
//
// Comparison operators

#include <concepts>
#include <__iterator/bounded_iter.h>

#include "test_iterators.h"
Expand Down Expand Up @@ -59,6 +60,16 @@ TEST_CONSTEXPR_CXX14 bool tests() {
assert(iter1 >= iter1);
}

#if TEST_STD_VER >= 20
if constexpr (requires(std::__bounded_iter<Iter> const iter) {
{ iter <=> iter } -> std::same_as<std::strong_ordering>;
}) {
// P1614
std::same_as<std::strong_ordering> decltype(auto) r1 = iter1 <=> iter2;
assert(r1 == std::strong_ordering::less);
}
#endif

return true;
}

Expand All @@ -69,8 +80,11 @@ int main(int, char**) {
#endif

#if TEST_STD_VER > 17
tests<contiguous_iterator<int*> >();
static_assert(tests<contiguous_iterator<int*> >(), "");
tests<contiguous_iterator<int*>>();
static_assert(tests<contiguous_iterator<int*>>());

tests<three_way_contiguous_iterator<int*>>();
static_assert(tests<three_way_contiguous_iterator<int*>>());
Comment on lines +82 to +83
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would declare

template <class Iter, bool HasSpaceship = false>
TEST_CONSTEXPR_CXX14 bool tests() {
  ...
  if constexpr (HasSpaceship) {
    ...
  }
}

Then:

  tests<three_way_contiguous_iterator<int*>, /* has spaceship */ true>();
  static_assert(tests<three_way_contiguous_iterator<int*>, /* has spaceship */ true>());

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this is no longer needed. This was a left-over while reworking the patch. operator<=> always works on these iterators.

#endif

return 0;
Expand Down
18 changes: 18 additions & 0 deletions libcxx/test/std/containers/sequences/array/iterators.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,15 @@ TEST_CONSTEXPR_CXX17 bool tests()
assert(std::rbegin(c) != std::rend(c));
assert(std::cbegin(c) != std::cend(c));
assert(std::crbegin(c) != std::crend(c));

# if TEST_STD_VER >= 20
// P1614 + LWG3352
std::same_as<std::strong_ordering> decltype(auto) r1 = ii1 <=> ii2;
assert(r1 == std::strong_ordering::equal);

std::same_as<std::strong_ordering> decltype(auto) r2 = cii <=> ii2;
assert(r2 == std::strong_ordering::equal);
# endif
}
{
typedef std::array<int, 0> C;
Expand Down Expand Up @@ -189,6 +198,15 @@ TEST_CONSTEXPR_CXX17 bool tests()
assert(std::rbegin(c) == std::rend(c));
assert(std::cbegin(c) == std::cend(c));
assert(std::crbegin(c) == std::crend(c));

# if TEST_STD_VER >= 20
// P1614 + LWG3352
std::same_as<std::strong_ordering> decltype(auto) r1 = ii1 <=> ii2;
assert(r1 == std::strong_ordering::equal);

std::same_as<std::strong_ordering> decltype(auto) r2 = cii <=> ii2;
assert(r2 == std::strong_ordering::equal);
# endif
}
}
#endif
Expand Down
29 changes: 29 additions & 0 deletions libcxx/test/std/containers/sequences/deque/iterators.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,27 @@ int main(int, char**)
i = c.begin();
C::const_iterator j;
j = c.cbegin();

assert(i == j);
assert(!(i != j));

assert(!(i < j));
assert((i <= j));

assert(!(i > j));
assert((i >= j));

# if TEST_STD_VER >= 20
// P1614 + LWG3352
// When the allocator does not have operator<=> then the iterator uses a
// fallback to provide operator<=>.
// Make sure to test with an allocator that does not have operator<=>.
static_assert(!std::three_way_comparable<min_allocator<int>, std::strong_ordering>);
static_assert(std::three_way_comparable<typename C::iterator, std::strong_ordering>);

std::same_as<std::strong_ordering> decltype(auto) r1 = i <=> j;
assert(r1 == std::strong_ordering::equal);
# endif
}
#endif
#if TEST_STD_VER > 11
Expand Down Expand Up @@ -74,6 +94,15 @@ int main(int, char**)
// assert ( cii != c.begin());
// assert ( cii != c.cend());
// assert ( ii1 != c.end());

# if TEST_STD_VER >= 20
// P1614 + LWG3352
std::same_as<std::strong_ordering> decltype(auto) r1 = ii1 <=> ii2;
assert(r1 == std::strong_ordering::equal);

std::same_as<std::strong_ordering> decltype(auto) r2 = cii <=> ii2;
assert(r2 == std::strong_ordering::equal);
# endif // TEST_STD_VER > 20
}
#endif

Expand Down
37 changes: 37 additions & 0 deletions libcxx/test/std/containers/sequences/vector.bool/iterators.pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,21 @@ TEST_CONSTEXPR_CXX20 bool tests()
C::iterator i = c.begin();
C::iterator j = c.end();
assert(std::distance(i, j) == 0);

assert(i == j);
assert(!(i != j));

assert(!(i < j));
assert((i <= j));

assert(!(i > j));
assert((i >= j));

# if TEST_STD_VER >= 20
// P1614 + LWG3352
std::same_as<std::strong_ordering> decltype(auto) r = i <=> j;
assert(r == std::strong_ordering::equal);
# endif
}
{
typedef bool T;
Expand All @@ -86,7 +100,21 @@ TEST_CONSTEXPR_CXX20 bool tests()
C::const_iterator i = c.begin();
C::const_iterator j = c.end();
assert(std::distance(i, j) == 0);

assert(i == j);
assert(!(i != j));

assert(!(i < j));
assert((i <= j));

assert(!(i > j));
assert((i >= j));

# if TEST_STD_VER >= 20
// P1614 + LWG3352
std::same_as<std::strong_ordering> decltype(auto) r = i <=> j;
assert(r == std::strong_ordering::equal);
# endif
}
{
typedef bool T;
Expand Down Expand Up @@ -131,6 +159,15 @@ TEST_CONSTEXPR_CXX20 bool tests()
assert ( (cii >= ii1 ));
assert (cii - ii1 == 0);
assert (ii1 - cii == 0);

# if TEST_STD_VER >= 20
// P1614 + LWG3352
std::same_as<std::strong_ordering> decltype(auto) r1 = ii1 <=> ii2;
assert(r1 == std::strong_ordering::equal);

std::same_as<std::strong_ordering> decltype(auto) r2 = cii <=> ii2;
assert(r2 == std::strong_ordering::equal);
# endif // TEST_STD_VER > 20
}
#endif

Expand Down
Loading
Loading