Skip to content

[libcxx] adds ranges::fold_left_with_iter and ranges::fold_left #75259

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 27 commits into from
Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
e36a06d
[libcxx] extends test range types to take a value-type
cjdb Dec 6, 2023
b373740
[libcxx] adds `ranges::fold_left_with_iter` and `ranges::fold_left`
cjdb Dec 13, 2023
e776610
changes to using static opertaor()
cjdb Dec 13, 2023
15b686b
adds missing export
cjdb Dec 14, 2023
bec1572
changes requirements test from asserts to verify
cjdb Dec 14, 2023
6a57523
adds missing file
cjdb Dec 14, 2023
12f36c4
repeats for `fold_left`
cjdb Dec 14, 2023
07ddbb4
Revert "changes requirements test from asserts to verify"
cjdb Dec 15, 2023
5376bec
Revert "adds missing file"
cjdb Dec 15, 2023
c806db0
Revert "repeats for `fold_left`"
cjdb Dec 15, 2023
5523376
consolidates requirements.compile.pass.cpp for the fold algos
cjdb Dec 15, 2023
5822e9d
eliminates unnecessary `__cpo` namespace
cjdb Dec 15, 2023
2e20461
reworks valid.cpp to incorporate feedback
cjdb Dec 18, 2023
b0e5f7c
adds documentation and fixes telemetry move
cjdb Dec 18, 2023
824e1fe
fixes telemetry move assignment
cjdb Dec 18, 2023
1b1b3d4
fixes in_value_result
cjdb Dec 19, 2023
141be97
replaces rogue integers with a telemetry struct
cjdb Dec 19, 2023
bdd4f59
Merge branch 'main' into fold_left
cjdb Dec 19, 2023
70e7764
applies clang-format
cjdb Dec 19, 2023
c8aa9e6
adds `_LIBCPP_HIDE_FROM_ABI` to `in_value_result`
cjdb Dec 19, 2023
33bc982
manually fixes clang-format issue
cjdb Dec 19, 2023
f68da2c
handles cxx26 action failure
cjdb Dec 19, 2023
15cc627
fixes CI for C++20
cjdb Dec 19, 2023
80bc73b
adds tests for pointer-to-member functions and renames test functions…
cjdb Dec 19, 2023
991db00
changes parameter name so it doesn't shadow
cjdb Dec 19, 2023
7bc4409
handle platforms that don't support certain features
cjdb Dec 19, 2023
2b44b37
handle platforms that don't support certain features
cjdb Dec 20, 2023
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: 6 additions & 0 deletions libcxx/docs/Status/RangesAlgorithms.csv
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,9 @@ C++23,`shift_right <https://wg21.link/p2440r1>`_,Unassigned,No patch yet,Not sta
C++23,`iota (algorithm) <https://wg21.link/p2440r1>`_,Unassigned,No patch yet,Not started
C++23,`fold <https://wg21.link/p2322r5>`_,Unassigned,No patch yet,Not started
C++23,`contains <https://wg21.link/p2302r2>`_,Zijun Zhao,No patch yet,In Progress
C++23,`fold_left_with_iter <https://wg21.link/p2322r6>`_,Christopher Di Bella,N/A,Complete
C++23,`fold_left <https://wg21.link/p2322r6>`_,Christopher Di Bella,N/A,Complete
C++23,`fold_left_first_with_iter <https://wg21.link/p2322r6>`_,Christopher Di Bella,N/A,In progress
C++23,`fold_left_first <https://wg21.link/p2322r6>`_,Christopher Di Bella,N/A,In progress
C++23,`fold_right <https://wg21.link/p2322r6>`_,Christopher Di Bella,N/A,In progress
C++23,`fold_right_last <https://wg21.link/p2322r6>`_,Christopher Di Bella,N/A,In progress
1 change: 1 addition & 0 deletions libcxx/include/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ set(files
__algorithm/find_if.h
__algorithm/find_if_not.h
__algorithm/find_segment_if.h
__algorithm/fold.h
__algorithm/for_each.h
__algorithm/for_each_n.h
__algorithm/for_each_segment.h
Expand Down
125 changes: 125 additions & 0 deletions libcxx/include/__algorithm/fold.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
// -*- C++ -*-
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef _LIBCPP___ALGORITHM_FOLD_H
#define _LIBCPP___ALGORITHM_FOLD_H

#include <__concepts/assignable.h>
#include <__concepts/convertible_to.h>
#include <__concepts/invocable.h>
#include <__concepts/movable.h>
#include <__config>
#include <__functional/invoke.h>
#include <__functional/reference_wrapper.h>
#include <__iterator/concepts.h>
#include <__iterator/iterator_traits.h>
#include <__iterator/next.h>
#include <__ranges/access.h>
#include <__ranges/concepts.h>
#include <__ranges/dangling.h>
#include <__type_traits/decay.h>
#include <__type_traits/invoke.h>
#include <__utility/forward.h>
#include <__utility/move.h>

#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
#endif

_LIBCPP_BEGIN_NAMESPACE_STD

#if _LIBCPP_STD_VER >= 23

namespace ranges {
template <class _Ip, class _Tp>
struct in_value_result {
_LIBCPP_NO_UNIQUE_ADDRESS _Ip in;
_LIBCPP_NO_UNIQUE_ADDRESS _Tp value;

template <class _I2, class _T2>
requires convertible_to<const _Ip&, _I2> && convertible_to<const _Tp&, _T2>
_LIBCPP_HIDE_FROM_ABI constexpr operator in_value_result<_I2, _T2>() const& {
return {in, value};
}

template <class _I2, class _T2>
requires convertible_to<_Ip, _I2> && convertible_to<_Tp, _T2>
_LIBCPP_HIDE_FROM_ABI constexpr operator in_value_result<_I2, _T2>() && {
return {std::move(in), std::move(value)};
}
};

template <class _Ip, class _Tp>
using fold_left_with_iter_result = in_value_result<_Ip, _Tp>;

template <class _Fp, class _Tp, class _Ip, class _Rp, class _Up = decay_t<_Rp>>
concept __indirectly_binary_left_foldable_impl =
convertible_to<_Rp, _Up> && //
movable<_Tp> && //
movable<_Up> && //
convertible_to<_Tp, _Up> && //
invocable<_Fp&, _Up, iter_reference_t<_Ip>> && //
assignable_from<_Up&, invoke_result_t<_Fp&, _Up, iter_reference_t<_Ip>>>;

template <class _Fp, class _Tp, class _Ip>
concept __indirectly_binary_left_foldable =
copy_constructible<_Fp> && //
invocable<_Fp&, _Tp, iter_reference_t<_Ip>> && //
__indirectly_binary_left_foldable_impl<_Fp, _Tp, _Ip, invoke_result_t<_Fp&, _Tp, iter_reference_t<_Ip>>>;

struct __fold_left_with_iter {
template <input_iterator _Ip, sentinel_for<_Ip> _Sp, class _Tp, __indirectly_binary_left_foldable<_Tp, _Ip> _Fp>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI static constexpr auto
operator()(_Ip __first, _Sp __last, _Tp __init, _Fp __f) {
using _Up = decay_t<invoke_result_t<_Fp&, _Tp, iter_reference_t<_Ip>>>;

if (__first == __last) {
return fold_left_with_iter_result<_Ip, _Up>{std::move(__first), _Up(std::move(__init))};
}

_Up __result = std::invoke(__f, std::move(__init), *__first);
for (++__first; __first != __last; ++__first) {
__result = std::invoke(__f, std::move(__result), *__first);
}

return fold_left_with_iter_result<_Ip, _Up>{std::move(__first), std::move(__result)};
}

template <input_range _Rp, class _Tp, __indirectly_binary_left_foldable<_Tp, iterator_t<_Rp>> _Fp>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI static constexpr auto operator()(_Rp&& __r, _Tp __init, _Fp __f) {
auto __result = operator()(ranges::begin(__r), ranges::end(__r), std::move(__init), std::ref(__f));

using _Up = decay_t<invoke_result_t<_Fp&, _Tp, range_reference_t<_Rp>>>;
return fold_left_with_iter_result<borrowed_iterator_t<_Rp>, _Up>{std::move(__result.in), std::move(__result.value)};
}
};

inline constexpr auto fold_left_with_iter = __fold_left_with_iter();

struct __fold_left {
template <input_iterator _Ip, sentinel_for<_Ip> _Sp, class _Tp, __indirectly_binary_left_foldable<_Tp, _Ip> _Fp>
Copy link
Member

Choose a reason for hiding this comment

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

Nit: we usually use longer names for template arguments in ranges (_Iter, _Sent, etc.), we should do the same here for consistency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's also code that uses _Ip and _Sp. I think it would be better to have a discussion about this outside of the review about whether it's worth unifying the names.

Copy link
Member

Choose a reason for hiding this comment

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

IIUC, we did have that discussion at the time, and I think _Iter and _Sent were the resolution.

Copy link
Member

Choose a reason for hiding this comment

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

@var-const I would be happy to review any proposal put forward for a standard naming convention in libc++.

Different parts do things differently, and this is very much inline with existing code in libc++. I don't think its appropriate to block this change on the broader conversation about names.

Copy link
Member

Choose a reason for hiding this comment

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

@EricWF This discussion has already happened, and I can confirm that the outcome of it was to use _Iter and _Sent. Maintaining consistency is one of the purposes of doing a code review, and I don't think the fact that our code base is not perfect in this regard allows dismissing that. If you would like to reopen that discussion, you are also quite welcome to come up with a new proposal.

Copy link
Member

Choose a reason for hiding this comment

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

@var-const Sounds good! That conversation already happened, there was consensus, and now there is no longer. I would be happy to help re-establish consensus in a more inclusive forum.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am okay with the current resolution given that a discussion has happened, and am happy to make a dedicated migration patch for all _Ip/_Sp names. This has not been applied in #76534: I'll make a dedicated commit for that in about an hour.

I think that the contribution guidelines should probably issue guidance on what kinds of names we expect, especially when it comes to names that we want to have some element of consistency for. Perhaps that could be the subject of the new discussion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #76540 for the renaming patch.

_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI static constexpr auto
operator()(_Ip __first, _Sp __last, _Tp __init, _Fp __f) {
return fold_left_with_iter(std::move(__first), std::move(__last), std::move(__init), std::ref(__f)).value;
}

template <input_range _Rp, class _Tp, __indirectly_binary_left_foldable<_Tp, iterator_t<_Rp>> _Fp>
_LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI static constexpr auto operator()(_Rp&& __r, _Tp __init, _Fp __f) {
return fold_left_with_iter(ranges::begin(__r), ranges::end(__r), std::move(__init), std::ref(__f)).value;
}
};

inline constexpr auto fold_left = __fold_left();
} // namespace ranges

#endif // _LIBCPP_STD_VER >= 23

_LIBCPP_END_NAMESPACE_STD

#endif // _LIBCPP___ALGORITHM_FOLD_H
21 changes: 21 additions & 0 deletions libcxx/include/algorithm
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ namespace ranges {
template <class I>
struct in_found_result; // since C++20

template <class I, class T>
struct in_value_result; // since C++23

template<forward_iterator I, sentinel_for<I> S, class Proj = identity,
indirect_strict_weak_order<projected<I, Proj>> Comp = ranges::less> // since C++20
constexpr I min_element(I first, S last, Comp comp = {}, Proj proj = {});
Expand Down Expand Up @@ -873,6 +876,23 @@ namespace ranges {
ranges::search_n(R&& r, range_difference_t<R> count,
const T& value, Pred pred = {}, Proj proj = {}); // since C++20

template<input_iterator I, sentinel_for<I> S, class T,
indirectly-binary-left-foldable<T, I> F>
constexpr auto ranges::fold_left(I first, S last, T init, F f); // since C++23

template<input_range R, class T, indirectly-binary-left-foldable<T, iterator_t<R>> F>
constexpr auto fold_left(R&& r, T init, F f); // since C++23

template<class I, class T>
using fold_left_with_iter_result = in_value_result<I, T>; // since C++23

template<input_iterator I, sentinel_for<I> S, class T,
indirectly-binary-left-foldable<T, I> F>
constexpr see below fold_left_with_iter(I first, S last, T init, F f); // since C++23

template<input_range R, class T, indirectly-binary-left-foldable<T, iterator_t<R>> F>
constexpr see below fold_left_with_iter(R&& r, T init, F f); // since C++23

template<forward_iterator I1, sentinel_for<I1> S1, forward_iterator I2, sentinel_for<I2> S2,
class Pred = ranges::equal_to, class Proj1 = identity, class Proj2 = identity>
requires indirectly_comparable<I1, I2, Pred, Proj1, Proj2>
Expand Down Expand Up @@ -1778,6 +1798,7 @@ template <class BidirectionalIterator, class Compare>
#include <__algorithm/find_first_of.h>
#include <__algorithm/find_if.h>
#include <__algorithm/find_if_not.h>
#include <__algorithm/fold.h>
#include <__algorithm/for_each.h>
#include <__algorithm/for_each_n.h>
#include <__algorithm/generate.h>
Expand Down
1 change: 1 addition & 0 deletions libcxx/include/module.modulemap.in
Original file line number Diff line number Diff line change
Expand Up @@ -666,6 +666,7 @@ module std_private_algorithm_find_first_of [system
module std_private_algorithm_find_if [system] { header "__algorithm/find_if.h" }
module std_private_algorithm_find_if_not [system] { header "__algorithm/find_if_not.h" }
module std_private_algorithm_find_segment_if [system] { header "__algorithm/find_segment_if.h" }
module std_private_algorithm_fold [system] { header "__algorithm/fold.h" }
module std_private_algorithm_for_each [system] { header "__algorithm/for_each.h" }
module std_private_algorithm_for_each_n [system] { header "__algorithm/for_each_n.h" }
module std_private_algorithm_for_each_segment [system] { header "__algorithm/for_each_segment.h" }
Expand Down
8 changes: 4 additions & 4 deletions libcxx/modules/std/algorithm.inc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export namespace std {
using std::ranges::in_in_result;
using std::ranges::in_out_out_result;
using std::ranges::in_out_result;
// using std::ranges::in_value_result;
using std::ranges::in_value_result;
using std::ranges::min_max_result;
// using std::ranges::out_value_result;
} // namespace ranges
Expand Down Expand Up @@ -157,15 +157,15 @@ export namespace std {
// [alg.ends.with], ends with
using std::ranges::ends_with;

# if 0
// [alg.fold], fold
using std::ranges::fold_left;
using std::ranges::fold_left_with_iter;
using std::ranges::fold_left_with_iter_result;
# if 0
using std::ranges::fold_left_first;
using std::ranges::fold_right;
using std::ranges::fold_right_last;
using std::ranges::fold_left_with_iter;
using std::ranges::fold_left_with_iter_result;
using std::ranges::fold_left_with_iter;
using std::ranges::fold_left_first_with_iter;
using std::ranges::fold_left_first_with_iter;
# endif
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

#include <algorithm>

#include "test_macros.h"

void test() {
int range[1];
int* iter = range;
Expand Down Expand Up @@ -87,4 +89,15 @@ void test() {
std::ranges::unique(iter, iter); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
std::ranges::upper_bound(range, 1); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
std::ranges::upper_bound(iter, iter, 1); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}

#if TEST_STD_VER >= 23
std::ranges::fold_left(range, 0, std::plus());
// expected-warning@-1{{ignoring return value of function declared with 'nodiscard' attribute}}
std::ranges::fold_left(iter, iter, 0, std::plus());
// expected-warning@-1{{ignoring return value of function declared with 'nodiscard' attribute}}
std::ranges::fold_left_with_iter(range, 0, std::plus());
// expected-warning@-1{{ignoring return value of function declared with 'nodiscard' attribute}}
std::ranges::fold_left_with_iter(iter, iter, 0, std::plus());
// expected-warning@-1{{ignoring return value of function declared with 'nodiscard' attribute}}
#endif
}
Loading