[libc++] Add `ranges::fold_left_first` and `ranges::fold_left_first_with_iter` #121558

JCGoran · 2025-01-03T10:43:42Z

Implementation mostly borrowed from #75259.

github-actions · 2025-01-03T10:44:01Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

frederick-vs-ja

Thanks! Mostly LGTM!

libcxx/include/__algorithm/ranges_fold.h

github-actions · 2025-01-03T13:33:09Z

✅ With the latest revision this PR passed the C/C++ code formatter.

libcxx/include/__algorithm/ranges_fold.h

llvmbot · 2025-01-07T16:33:43Z

@llvm/pr-subscribers-libcxx

Author: None (JCGoran)

Changes

Implementation mostly borrowed from #75259.
Probably needs more tests, hence just a draft for now.

Full diff: https://github.com/llvm/llvm-project/pull/121558.diff

5 Files Affected:

(modified) libcxx/include/__algorithm/ranges_fold.h (+53)
(modified) libcxx/include/algorithm (+21)
(modified) libcxx/modules/std/algorithm.inc (+3-4)
(modified) libcxx/test/libcxx/diagnostics/algorithm.nodiscard.verify.cpp (+4)
(modified) libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/left_folds.pass.cpp (+58)

diff --git a/libcxx/include/__algorithm/ranges_fold.h b/libcxx/include/__algorithm/ranges_fold.h
index d2c39213985044..b94fb59f6090a9 100644
--- a/libcxx/include/__algorithm/ranges_fold.h
+++ b/libcxx/include/__algorithm/ranges_fold.h
@@ -28,6 +28,7 @@
 #include <__type_traits/invoke.h>
 #include <__utility/forward.h>
 #include <__utility/move.h>
+#include <optional>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -62,6 +63,9 @@ struct in_value_result {
 template <class _Ip, class _Tp>
 using fold_left_with_iter_result = in_value_result<_Ip, _Tp>;
 
+template <class _Ip, class _Tp>
+using fold_left_first_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> &&                    //
@@ -118,6 +122,55 @@ struct __fold_left {
 };
 
 inline constexpr auto fold_left = __fold_left();
+
+struct __fold_left_first_with_iter {
+  template <input_iterator _Ip, sentinel_for<_Ip> _Sp, __indirectly_binary_left_foldable<iter_value_t<_Ip>, _Ip> _Fp>
+    requires constructible_from<iter_value_t<_Ip>, iter_reference_t<_Ip>>
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI static constexpr auto operator()(_Ip __first, _Sp __last, _Fp __f) {
+    using _Up = decltype(fold_left(std::move(__first), __last, iter_value_t<_Ip>(*__first), __f));
+
+    // case of empty range
+    if (__first == __last) {
+      return fold_left_first_with_iter_result<_Ip, optional<_Up>>{std::move(__first), optional<_Up>()};
+    }
+
+    _Up __result(*__first);
+    for (++__first; __first != __last; ++__first) {
+      __result = std::invoke(__f, std::move(__result), *__first);
+    }
+
+    return fold_left_first_with_iter_result<_Ip, optional<_Up>>{std::move(__first), optional<_Up>(std::move(__result))};
+  }
+
+  template <input_range _Rp, __indirectly_binary_left_foldable<range_value_t<_Rp>, iterator_t<_Rp>> _Fp>
+    requires constructible_from<range_value_t<_Rp>, range_reference_t<_Rp>>
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI static constexpr auto operator()(_Rp&& __r, _Fp __f) {
+    auto __result = operator()(ranges::begin(__r), ranges::end(__r), std::ref(__f));
+
+    using _Up = decltype(fold_left(ranges::begin(__r), ranges::end(__r), range_value_t<_Rp>(*ranges::begin(__r)), __f));
+    return fold_left_first_with_iter_result<borrowed_iterator_t<_Rp>, optional<_Up>>{
+        std::move(__result.in), std::move(__result.value)};
+  }
+};
+
+inline constexpr auto fold_left_first_with_iter = __fold_left_first_with_iter();
+
+struct __fold_left_first {
+  template <input_iterator _Ip, sentinel_for<_Ip> _Sp, __indirectly_binary_left_foldable<iter_value_t<_Ip>, _Ip> _Fp>
+    requires constructible_from<iter_value_t<_Ip>, iter_reference_t<_Ip>>
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI static constexpr auto operator()(_Ip __first, _Sp __last, _Fp __f) {
+    return fold_left_first_with_iter(std::move(__first), std::move(__last), std::ref(__f)).value;
+  }
+
+  template <input_range _Rp, __indirectly_binary_left_foldable<range_value_t<_Rp>, iterator_t<_Rp>> _Fp>
+    requires constructible_from<range_value_t<_Rp>, range_reference_t<_Rp>>
+  [[nodiscard]] _LIBCPP_HIDE_FROM_ABI static constexpr auto operator()(_Rp&& __r, _Fp __f) {
+    return fold_left_first_with_iter(ranges::begin(__r), ranges::end(__r), std::ref(__f)).value;
+  }
+};
+
+inline constexpr auto fold_left_first = __fold_left_first();
+
 } // namespace ranges
 
 #endif // _LIBCPP_STD_VER >= 23
diff --git a/libcxx/include/algorithm b/libcxx/include/algorithm
index e593ae26ed6e24..5592387e454950 100644
--- a/libcxx/include/algorithm
+++ b/libcxx/include/algorithm
@@ -938,6 +938,15 @@ namespace ranges {
   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<input_iterator I, sentinel_for<I> S,
+           indirectly-binary-left-foldable<T, I> F>
+    requires constructible_from<iter_value_t<I>, iter_reference_t<I>>
+    constexpr see below fold_left_first(I first, S last, F f);                                      // since C++23
+
+  template<input_range R, indirectly-binary-left-foldable<T, iterator_t<R>> F>
+    requires constructible_from<range_value_t<R>, range_reference_t<R>>
+    constexpr see below fold_left_first(R&& r, F f);                                                // since C++23
+
   template<class I, class T>
     using fold_left_with_iter_result = in_value_result<I, T>;                                       // since C++23
 
@@ -948,6 +957,18 @@ namespace ranges {
   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<class I, class T>
+    using fold_left_first_with_iter_result = in_value_result<I, T>;                                 // since C++23
+
+  template<input_iterator I, sentinel_for<I> S,
+           indirectly-binary-left-foldable<T, I> F>
+    requires constructible_from<iter_value_t<I>, iter_reference_t<I>>
+    constexpr see below fold_left_first_with_iter(I first, S last, F f);                            // since C++23
+
+  template<input_range R, indirectly-binary-left-foldable<T, iterator_t<R>> F>
+    requires constructible_from<range_value_t<R>, range_reference_t<R>>
+    constexpr see below fold_left_first_with_iter(R&& r, 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>
diff --git a/libcxx/modules/std/algorithm.inc b/libcxx/modules/std/algorithm.inc
index 3c2139cd64ee49..20a3aea4fa8dab 100644
--- a/libcxx/modules/std/algorithm.inc
+++ b/libcxx/modules/std/algorithm.inc
@@ -162,15 +162,14 @@ export namespace std {
 
     // [alg.fold], fold
     using std::ranges::fold_left;
+    using std::ranges::fold_left_first;
+    using std::ranges::fold_left_first_with_iter;
+    using std::ranges::fold_left_first_with_iter_result;
     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_first_with_iter;
-    using std::ranges::fold_left_first_with_iter;
 #  endif
 #endif // _LIBCPP_STD_VER >= 23
   }    // namespace ranges
diff --git a/libcxx/test/libcxx/diagnostics/algorithm.nodiscard.verify.cpp b/libcxx/test/libcxx/diagnostics/algorithm.nodiscard.verify.cpp
index 14febc12a8a2d9..f4f14e513308ba 100644
--- a/libcxx/test/libcxx/diagnostics/algorithm.nodiscard.verify.cpp
+++ b/libcxx/test/libcxx/diagnostics/algorithm.nodiscard.verify.cpp
@@ -392,5 +392,9 @@ void test() {
   // 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}}
+  std::ranges::fold_left_first_with_iter(range, std::plus());
+  // expected-warning@-1{{ignoring return value of function declared with 'nodiscard' attribute}}
+  std::ranges::fold_left_first_with_iter(iter, iter, std::plus());
+  // expected-warning@-1{{ignoring return value of function declared with 'nodiscard' attribute}}
 #endif
 }
diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/left_folds.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/left_folds.pass.cpp
index 4987ca9cac4ae0..c5b75a2e4ca3c2 100644
--- a/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/left_folds.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/left_folds.pass.cpp
@@ -39,6 +39,7 @@
 #include <string_view>
 #include <string>
 #include <vector>
+#include <optional>
 
 #include "test_macros.h"
 #include "test_range.h"
@@ -50,6 +51,8 @@
 #endif
 
 using std::ranges::fold_left;
+using std::ranges::fold_left_first;
+using std::ranges::fold_left_first_with_iter;
 using std::ranges::fold_left_with_iter;
 
 template <class Result, class Range, class T>
@@ -105,6 +108,48 @@ constexpr void check_iterator(R& r, T const& init, F f, Expected const& expected
   }
 }
 
+template <std::ranges::input_range R, class F, std::equality_comparable Expected>
+  requires std::copyable<R>
+constexpr void check_iterator(R& r, F f, std::optional<Expected> const& expected) {
+  {
+    is_in_value_result<R, std::optional<Expected>> decltype(auto) result =
+        fold_left_first_with_iter(r.begin(), r.end(), f);
+    assert(result.in == r.end());
+    assert(result.value == expected);
+  }
+
+  {
+    auto telemetry = invocable_telemetry();
+    auto f2        = invocable_with_telemetry(f, telemetry);
+    is_in_value_result<R, std::optional<Expected>> decltype(auto) result =
+        fold_left_first_with_iter(r.begin(), r.end(), f2);
+    assert(result.in == r.end());
+    assert(result.value == expected);
+    if (result.value.has_value()) {
+      assert(telemetry.invocations == std::ranges::distance(r) - 1);
+      assert(telemetry.moves == 0);
+      assert(telemetry.copies == 1);
+    }
+  }
+
+  {
+    std::same_as<std::optional<Expected>> decltype(auto) result = fold_left_first(r.begin(), r.end(), f);
+    assert(result == expected);
+  }
+
+  {
+    auto telemetry                                              = invocable_telemetry();
+    auto f2                                                     = invocable_with_telemetry(f, telemetry);
+    std::same_as<std::optional<Expected>> decltype(auto) result = fold_left_first(r.begin(), r.end(), f2);
+    assert(result == expected);
+    if (result.has_value()) {
+      assert(telemetry.invocations == std::ranges::distance(r) - 1);
+      assert(telemetry.moves == 0);
+      assert(telemetry.copies == 1);
+    }
+  }
+}
+
 template <std::ranges::input_range R, class T, class F, std::equality_comparable Expected>
   requires std::copyable<R>
 constexpr void check_lvalue_range(R& r, T const& init, F f, Expected const& expected) {
@@ -186,19 +231,29 @@ constexpr void check(R r, T const& init, F f, Expected const& expected) {
   check_rvalue_range(r, init, f, expected);
 }
 
+template <std::ranges::input_range R, class F, std::equality_comparable Expected>
+  requires std::copyable<R>
+constexpr void check(R r, F f, std::optional<Expected> const& expected) {
+  check_iterator(r, f, expected);
+}
+
 constexpr void empty_range_test_case() {
   auto const data = std::vector<int>{};
   check(data, 100, std::plus(), 100);
   check(data, -100, std::multiplies(), -100);
+  check(data, std::plus(), std::optional<int>());
 
   check(data | std::views::take_while([](auto) { return false; }), 1.23, std::plus(), 1.23);
   check(data, Integer(52), &Integer::plus, Integer(52));
+  check(data | std::views::take_while([](auto) { return false; }), std::plus(), std::optional<int>());
 }
 
 constexpr void common_range_test_case() {
   auto const data = std::vector<int>{1, 2, 3, 4};
   check(data, 0, std::plus(), triangular_sum(data));
   check(data, 1, std::multiplies(), factorial(data.back()));
+  check(data, std::plus(), std::optional(triangular_sum(data)));
+  check(data, std::multiplies(), std::optional(factorial(data.back())));
 
   auto multiply_with_prev = [n = 1](auto const x, auto const y) mutable {
     auto const result = x * y * n;
@@ -206,6 +261,7 @@ constexpr void common_range_test_case() {
     return static_cast<std::size_t>(result);
   };
   check(data, 1, multiply_with_prev, factorial(data.size()) * factorial(data.size() - 1));
+  check(data, multiply_with_prev, std::optional(factorial(data.size()) * factorial(data.size() - 1)));
 
   auto fib = [n = 1](auto x, auto) mutable {
     auto old_x = x;
@@ -237,6 +293,7 @@ constexpr void non_common_range_test_case() {
     auto data  = std::vector<std::string>{"five", "three", "two", "six", "one", "four"};
     auto range = data | std::views::transform(parse);
     check(range, 0, std::plus(), triangular_sum(range));
+    check(range, std::plus(), std::optional(triangular_sum(range)));
   }
 
   {
@@ -248,6 +305,7 @@ constexpr void non_common_range_test_case() {
     auto range =
         std::views::lazy_split(data, ' ') | std::views::transform(to_string_view) | std::views::transform(parse);
     check(range, 0, std::plus(), triangular_sum(range));
+    check(range, std::plus(), std::optional(triangular_sum(range)));
   }
 }

JCGoran · 2025-02-05T18:17:25Z

Ping

JCGoran · 2025-02-15T12:34:44Z

Ping

JCGoran · 2025-02-25T13:52:48Z

Ping

frederick-vs-ja · 2025-02-25T16:41:42Z

I think we're very near to landing this. Let's add notes to libcxx/docs/ReleaseNotes/21.rst.

frederick-vs-ja

Tentatively approving. I pushed changes which added release notes to the documentation. @JCGoran Please confirm whether these notes are fine.

mordante

Thanks for working on this. In general it look good.

libcxx/docs/ReleaseNotes/21.rst

libcxx/docs/Status/Cxx23Papers.csv

libcxx/test/libcxx/diagnostics/algorithm.nodiscard.verify.cpp

libcxx/include/__algorithm/ranges_fold.h

mordante · 2025-03-19T18:37:27Z

libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/left_folds.pass.cpp

+using std::ranges::fold_left_first;
+using std::ranges::fold_left_first_with_iter;


We prefer not to do this.

Also we prefer to have one test per function overload.
For tests of a set of properties we sometimes use one test like for the nodiscard test.

Now I've split this file into four algorithms, each for one algorithm.

I'm not sure what should do next... Should we cover different overloads in different function call entries, or in different test files?

mordante

This looks good after addressing my last review comment.
Please apply that comment to all tests in this patch.

I like to have a quick look at the final tests.

mordante · 2025-03-21T17:33:44Z

libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/ranges.fold_left_first.pass.cpp

+constexpr void common_range_test_case() {
+  auto const data = std::vector<int>{1, 2, 3, 4};
+  check(data, std::plus(), std::optional(triangular_sum(data)));
+  check(data, std::multiplies(), std::optional(factorial(data.back())));


Sorry I missed this in the previous review. But I feel these tests are way too clever.

Suggested change

check(data, std::multiplies(), std::optional(factorial(data.back())));

check(data, std::multiplies(), std::optional(24));

makes the test a lot easier to inspect. It also allows to test the multiplies in the transform test below

auto data = std::vector<std::string>{"five", "three", "two", "six", "one", "four"}; auto range = data | std::views::transform(parse); check(range, std::plus(), std::optional(triangular_sum(range))); check(data, std::multiplies(), std::optional(720)); // <<< This is now tested.

philnik777 · 2025-03-25T07:53:07Z

libcxx/include/__algorithm/ranges_fold.h

@@ -82,14 +86,12 @@ struct __fold_left_with_iter {
  [[nodiscard]] _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) {
+    if (__first == __last)


Please avoid unrelated changes.

Zingam · 2025-04-02T10:08:37Z

@JCGoran IMO it makes sense to create a sub-issue in #105208 and use the GitHub syntax to associate the PR with it: https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue

JCGoran added 4 commits January 2, 2025 18:58

Initial impl of fold_left_first_with_iter

6359ec7

Update

8eb9f22

Update header

8067ea1

Formatting

a5515db

frederick-vs-ja reviewed Jan 3, 2025

View reviewed changes

libcxx/include/__algorithm/ranges_fold.h Outdated Show resolved Hide resolved

libcxx/include/__algorithm/ranges_fold.h Outdated Show resolved Hide resolved

JCGoran added 2 commits January 3, 2025 15:35

More formatting

1cdc9bf

Apply suggested changes

a5126a4

frederick-vs-ja reviewed Jan 5, 2025

View reviewed changes

libcxx/include/__algorithm/ranges_fold.h Show resolved Hide resolved

Add missing header

3816c4c

JCGoran marked this pull request as ready for review January 7, 2025 16:33

JCGoran requested a review from a team as a code owner January 7, 2025 16:33

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jan 7, 2025

frederick-vs-ja requested review from ldionne and philnik777 February 25, 2025 16:40

frederick-vs-ja requested review from mordante, cjdb and huixie90 February 26, 2025 01:02

frederick-vs-ja self-assigned this Mar 19, 2025

frederick-vs-ja added 2 commits March 19, 2025 17:03

Merge branch 'main' into jelic/fold_left_first

41c571d

Add release notes

7bac67a

frederick-vs-ja approved these changes Mar 19, 2025

View reviewed changes

frederick-vs-ja changed the title ~~[libc++] Add ranges::fold_left_first and ranges::fold_left_first_with_iter~~ [libc++] Add ranges::fold_left_first and ranges::fold_left_first_with_iter Mar 19, 2025

mordante requested changes Mar 19, 2025

View reviewed changes

mordante self-assigned this Mar 19, 2025

frederick-vs-ja added 7 commits March 21, 2025 16:59

Adjust release notes per review comments

abf6df7

Modify general test coverage for ranges algorithms

2da3541

Code style change in <__algorithm/ranges_fold.h>

8396d9e

Split left_folds.pass.cpp into four files; each for an algorithm

9ea524d

Merge branch 'main' into jelic/fold_left_first

92bdfba

Missed clang-formatting

ef107b7

Fixup niebloid.compile.pass.cpp

d17d852

mordante requested changes Mar 21, 2025

View reviewed changes

philnik777 reviewed Mar 25, 2025

View reviewed changes

		using std::ranges::fold_left_first;
		using std::ranges::fold_left_first_with_iter;

	check(data, std::multiplies(), std::optional(factorial(data.back())));
	check(data, std::multiplies(), std::optional(24));

[libc++] Add ranges::fold_left_first and ranges::fold_left_first_with_iter #121558

Are you sure you want to change the base?

[libc++] Add ranges::fold_left_first and ranges::fold_left_first_with_iter #121558

Conversation

JCGoran commented Jan 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 3, 2025

Uh oh!

frederick-vs-ja left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jan 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Jan 7, 2025

Uh oh!

JCGoran commented Feb 5, 2025

Uh oh!

JCGoran commented Feb 15, 2025

Uh oh!

JCGoran commented Feb 25, 2025

Uh oh!

frederick-vs-ja commented Feb 25, 2025

Uh oh!

frederick-vs-ja left a comment

Choose a reason for hiding this comment

Uh oh!

mordante left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mordante Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

frederick-vs-ja Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

frederick-vs-ja Mar 22, 2025

Choose a reason for hiding this comment

Uh oh!

mordante left a comment

Choose a reason for hiding this comment

Uh oh!

mordante Mar 21, 2025

Choose a reason for hiding this comment

Uh oh!

philnik777 Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

Zingam commented Apr 2, 2025

Uh oh!

Uh oh!

[libc++] Add `ranges::fold_left_first` and `ranges::fold_left_first_with_iter` #121558

[libc++] Add `ranges::fold_left_first` and `ranges::fold_left_first_with_iter` #121558

JCGoran commented Jan 3, 2025 •

edited

Loading

github-actions bot commented Jan 3, 2025 •

edited

Loading