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

[libc++][ranges] use static operator() for C++23 ranges #86052

Merged
merged 5 commits into from
Mar 22, 2024
Merged

[libc++][ranges] use static operator() for C++23 ranges #86052

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a111f93
[libc++][ranges] use 'static operator()' for C++23 ranges
Mar 21, 2024
a218649
[libc++][ranges] 'static operator()' for C++23 ranges
Mar 21, 2024
b38ba75
[libc++][ranges] 'static operator()' for C++23 ranges
Mar 21, 2024
4dbd6a3
[libc++][ranges] 'static operator()' for C++23 ranges
Mar 21, 2024
d6190fe
[libc++][ranges] 'static operator()' for C++23 ranges
Mar 22, 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
6 changes: 3 additions & 3 deletions libcxx/include/__algorithm/ranges_ends_with.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ namespace ranges {
namespace __ends_with {
struct __fn {
template <class _Iter1, class _Sent1, class _Iter2, class _Sent2, class _Pred, class _Proj1, class _Proj2>
static _LIBCPP_HIDE_FROM_ABI constexpr bool __ends_with_fn_impl_bidirectional(
_LIBCPP_HIDE_FROM_ABI static constexpr bool __ends_with_fn_impl_bidirectional(
_Iter1 __first1,
_Sent1 __last1,
_Iter2 __first2,
Expand All @@ -56,7 +56,7 @@ struct __fn {
}

template <class _Iter1, class _Sent1, class _Iter2, class _Sent2, class _Pred, class _Proj1, class _Proj2>
static _LIBCPP_HIDE_FROM_ABI constexpr bool __ends_with_fn_impl(
_LIBCPP_HIDE_FROM_ABI static constexpr bool __ends_with_fn_impl(
_Iter1 __first1,
_Sent1 __last1,
_Iter2 __first2,
Expand All @@ -65,7 +65,7 @@ struct __fn {
_Proj1& __proj1,
_Proj2& __proj2) {
if constexpr (std::bidirectional_iterator<_Sent1> && std::bidirectional_iterator<_Sent2> &&
(!std::random_access_iterator<_Sent1>)&&(!std::random_access_iterator<_Sent2>)) {
(!std::random_access_iterator<_Sent1>) && (!std::random_access_iterator<_Sent2>)) {
return __ends_with_fn_impl_bidirectional(__first1, __last1, __first2, __last2, __pred, __proj1, __proj2);

} else {
Expand Down
8 changes: 4 additions & 4 deletions libcxx/include/__algorithm/ranges_starts_with.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ struct __fn {
class _Proj1 = identity,
class _Proj2 = identity>
requires indirectly_comparable<_Iter1, _Iter2, _Pred, _Proj1, _Proj2>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr bool operator()(
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI static constexpr bool operator()(
_Iter1 __first1,
_Sent1 __last1,
_Iter2 __first2,
_Sent2 __last2,
_Pred __pred = {},
_Proj1 __proj1 = {},
_Proj2 __proj2 = {}) const {
_Proj2 __proj2 = {}) {
return __mismatch::__fn::__go(
std::move(__first1),
std::move(__last1),
Expand All @@ -67,8 +67,8 @@ struct __fn {
class _Proj1 = identity,
class _Proj2 = identity>
requires indirectly_comparable<iterator_t<_Range1>, iterator_t<_Range2>, _Pred, _Proj1, _Proj2>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr bool operator()(
_Range1&& __range1, _Range2&& __range2, _Pred __pred = {}, _Proj1 __proj1 = {}, _Proj2 __proj2 = {}) const {
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI static constexpr bool
operator()(_Range1&& __range1, _Range2&& __range2, _Pred __pred = {}, _Proj1 __proj1 = {}, _Proj2 __proj2 = {}) {
return __mismatch::__fn::__go(
ranges::begin(__range1),
ranges::end(__range1),
Expand Down
16 changes: 8 additions & 8 deletions libcxx/include/__ranges/as_rvalue_view.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,18 +111,18 @@ namespace views {
namespace __as_rvalue {
struct __fn : __range_adaptor_closure<__fn> {
template <class _Range>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Range&& __range) const
noexcept(noexcept(/**/ as_rvalue_view(std::forward<_Range>(__range))))
-> decltype(/*--*/ as_rvalue_view(std::forward<_Range>(__range))) {
return /*-------------*/ as_rvalue_view(std::forward<_Range>(__range));
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI static constexpr auto
operator()(_Range&& __range) noexcept(noexcept(as_rvalue_view(std::forward<_Range>(__range))))
-> decltype(/*--------------------------*/ as_rvalue_view(std::forward<_Range>(__range))) {
return /*---------------------------------*/ as_rvalue_view(std::forward<_Range>(__range));
}

template <class _Range>
requires same_as<range_rvalue_reference_t<_Range>, range_reference_t<_Range>>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Range&& __range) const
noexcept(noexcept(/**/ views::all(std::forward<_Range>(__range))))
-> decltype(/*--*/ views::all(std::forward<_Range>(__range))) {
return /*-------------*/ views::all(std::forward<_Range>(__range));
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI static constexpr auto
operator()(_Range&& __range) noexcept(noexcept(views::all(std::forward<_Range>(__range))))
-> decltype(/*--------------------------*/ views::all(std::forward<_Range>(__range))) {
return /*---------------------------------*/ views::all(std::forward<_Range>(__range));
}
};
} // namespace __as_rvalue
Expand Down
16 changes: 9 additions & 7 deletions libcxx/include/__ranges/chunk_by_view.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,17 +205,19 @@ namespace views {
namespace __chunk_by {
struct __fn {
template <class _Range, class _Pred>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Range&& __range, _Pred&& __pred) const
noexcept(noexcept(/**/ chunk_by_view(std::forward<_Range>(__range), std::forward<_Pred>(__pred))))
-> decltype(/*--*/ chunk_by_view(std::forward<_Range>(__range), std::forward<_Pred>(__pred))) {
return /*-------------*/ chunk_by_view(std::forward<_Range>(__range), std::forward<_Pred>(__pred));
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI static constexpr auto operator()(
_Range&& __range,
_Pred&& __pred) noexcept(noexcept(chunk_by_view(std::forward<_Range>(__range), std::forward<_Pred>(__pred))))
-> decltype(/*-----------------*/ chunk_by_view(std::forward<_Range>(__range), std::forward<_Pred>(__pred))) {
return /*------------------------*/ chunk_by_view(std::forward<_Range>(__range), std::forward<_Pred>(__pred));
}

template <class _Pred>
requires constructible_from<decay_t<_Pred>, _Pred>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Pred&& __pred) const
noexcept(is_nothrow_constructible_v<decay_t<_Pred>, _Pred>) {
return __range_adaptor_closure_t(std::__bind_back(*this, std::forward<_Pred>(__pred)));
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI static constexpr auto
operator()(_Pred&& __pred) noexcept(is_nothrow_constructible_v<decay_t<_Pred>, _Pred>) {
constexpr auto __self = __fn{};
return __range_adaptor_closure_t(std::__bind_back(__self, std::forward<_Pred>(__pred)));
Copy link
Member Author

@xiaoyang-sde xiaoyang-sde Mar 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

  • std::__bind_back(*this, std::forward<_Pred>(__pred)) won't compile because this is not available in a static function.
  • std::__bind_back(operator(), std::forward<_Pred>(__pred)) won't compile because the compiler can't determine which overload of operator() to use.

The solution here is to initialize the function object __fn as a constexpr variable, which will be evaluated at compile time, and pass it to std::__bind_back. Please let me know if there's a better solution!

Copy link
Contributor

Choose a reason for hiding this comment

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

The solution here is to initialize the function object __fn as a constexpr variable, which will be evaluated at compile time, and pass it to std::__bind_back. Please let me know if there's a better solution!

Perhaps we can "pass" the functor as a template argument in the same way as P2714R1 (adopted for C++26).

Copy link
Member

Choose a reason for hiding this comment

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

I wonder whether we should change this place. We need a *this pointer so what is the benefit of this change?

Copy link
Member Author

@xiaoyang-sde xiaoyang-sde Mar 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I wonder whether we should change this place. We need a *this pointer so what is the benefit of this change?

This change allows the first overload of operator() to be static. We just need to figure out how to pass __fn to std::__bind_back here.

If P2714R1 is implemented, I think we can write it like this: (credit @frederick-vs-ja)

- constexpr auto __self = __fn{};
- return __range_adaptor_closure_t(std::__bind_back(__self, std::forward<_Pred>(__pred)));
+ return __range_adaptor_closure_t(std::__bind_back<__fn{}>(std::forward<_Pred>(__pred)));

Copy link
Member

Choose a reason for hiding this comment

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

We can't this is a C++23 feature and P2714 is a C++26 feature.

Copy link
Member Author

@xiaoyang-sde xiaoyang-sde Mar 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@philnik777 @frederick-vs-ja What do you think?

Context: philnik777 has initiated this pull requset and frederick-vs-ja has implemented a similar change on Microsoft STL (microsoft/STL#4358)

Copy link
Contributor

@frederick-vs-ja frederick-vs-ja Mar 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

If P2714R1 is implemented

We can't this is a C++23 feature and P2714 is a C++26 feature.

Oh, but IIUC we don't need to be waiting for P2714R1 being implemented. __bind_back is not a standard feature and it seems OK to make it equivalent to C++26 std::bind_back in C++20 mode.

Copy link
Member

Choose a reason for hiding this comment

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

The __bind_back implementation detail is used C++20 so that "runs" into the same issue. I really want to avoid this change in this PR. We can make a separate PR where we add a __bind_back for C++23 which does that. However in that case I want to see a measurable performance improvement before adding that.

Copy link
Member Author

Choose a reason for hiding this comment

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

The __bind_back implementation detail is used C++20 so that "runs" into the same issue. I really want to avoid this change in this PR. We can make a separate PR where we add a __bind_back for C++23 which does that. However in that case I want to see a measurable performance improvement before adding that.

Sounds good to me. Thanks for reviewing this PR!

Copy link
Member Author

Choose a reason for hiding this comment

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

The changes on this file have been reverted.

}
};
} // namespace __chunk_by
Expand Down
5 changes: 2 additions & 3 deletions libcxx/include/__ranges/repeat_view.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,14 +229,13 @@ namespace views {
namespace __repeat {
struct __fn {
template <class _Tp>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Tp&& __value) const
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI static constexpr auto operator()(_Tp&& __value)
noexcept(noexcept(ranges::repeat_view(std::forward<_Tp>(__value))))
-> decltype( ranges::repeat_view(std::forward<_Tp>(__value)))
{ return ranges::repeat_view(std::forward<_Tp>(__value)); }


template <class _Tp, class _Bound>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Tp&& __value, _Bound&& __bound_sentinel) const
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI static constexpr auto operator()(_Tp&& __value, _Bound&& __bound_sentinel)
noexcept(noexcept(ranges::repeat_view(std::forward<_Tp>(__value), std::forward<_Bound>(__bound_sentinel))))
-> decltype( ranges::repeat_view(std::forward<_Tp>(__value), std::forward<_Bound>(__bound_sentinel)))
{ return ranges::repeat_view(std::forward<_Tp>(__value), std::forward<_Bound>(__bound_sentinel)); }
Expand Down
4 changes: 2 additions & 2 deletions libcxx/include/__ranges/to.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr auto to(_Args&&... __args)
static_assert(
!is_volatile_v<_Container>, "The target container cannot be volatile-qualified, please remove the volatile");

auto __to_func = []<input_range _Range, class... _Tail>(_Range&& __range, _Tail&&... __tail)
auto __to_func = []<input_range _Range, class... _Tail>(_Range&& __range, _Tail&&... __tail) static
requires requires { //
/**/ ranges::to<_Container>(std::forward<_Range>(__range), std::forward<_Tail>(__tail)...);
}
Expand All @@ -223,7 +223,7 @@ _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr auto to(_Args&&... __args)
// clang-format off
auto __to_func = []<input_range _Range, class... _Tail,
class _DeducedExpr = typename _Deducer<_Container, _Range, _Tail...>::type>
(_Range&& __range, _Tail&& ... __tail)
(_Range&& __range, _Tail&& ... __tail) static
requires requires { //
/**/ ranges::to<_DeducedExpr>(std::forward<_Range>(__range), std::forward<_Tail>(__tail)...);
}
Expand Down
10 changes: 6 additions & 4 deletions libcxx/include/__ranges/zip_view.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,12 +489,14 @@ namespace views {
namespace __zip {

struct __fn {
_LIBCPP_HIDE_FROM_ABI constexpr auto operator()() const noexcept { return empty_view<tuple<>>{}; }
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI static constexpr auto operator()() noexcept {
return empty_view<tuple<>>{};
}

template <class... _Ranges>
_LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Ranges&&... __rs) const
noexcept(noexcept(zip_view<all_t<_Ranges&&>...>(std::forward<_Ranges>(__rs)...)))
-> decltype(zip_view<all_t<_Ranges&&>...>(std::forward<_Ranges>(__rs)...)) {
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI static constexpr auto
xiaoyang-sde marked this conversation as resolved.
Show resolved Hide resolved
operator()(_Ranges&&... __rs) noexcept(noexcept(zip_view<all_t<_Ranges&&>...>(std::forward<_Ranges>(__rs)...)))
-> decltype(zip_view<all_t<_Ranges&&>...>(std::forward<_Ranges>(__rs)...)) {
return zip_view<all_t<_Ranges>...>(std::forward<_Ranges>(__rs)...);
}
};
Expand Down
Loading