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

cjdb · 2023-12-13T00:19:07Z

Notable things in this commit does:

refactors __indirect_binary_left_foldable, making it slightly different (but equivalent) to indirect-binary-left-foldable, which improves readability (a patch to the Working Paper was made)
omits __cpo namespace, since it is not required for implementing niebloids (a cleanup should happen in 2024)
puts tests ensuring invocable robustness and dangling correctness inside the correctness testing to ensure that the algorithms' results are still correct

This increases the types' versatility so that they're not restricted just to `int*`.

llvmbot · 2023-12-13T00:19:23Z

@llvm/pr-subscribers-libcxx

Author: Christopher Di Bella (cjdb)

Changes

This commit depends on #74672.

Patch is 47.49 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/75259.diff

17 Files Affected:

(modified) libcxx/include/CMakeLists.txt (+1)
(added) libcxx/include/__algorithm/fold.h (+118)
(modified) libcxx/include/algorithm (+1)
(modified) libcxx/include/module.modulemap.in (+1)
(modified) libcxx/test/libcxx/diagnostics/ranges.nodiscard_extensions.verify.cpp (+13)
(added) libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/requirements.compile.pass.cpp (+99)
(added) libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/return_types.compile.pass.cpp (+171)
(added) libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/valid.pass.cpp (+93)
(added) libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left_with_iter/requirements.compile.pass.cpp (+107)
(added) libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left_with_iter/return_types.compile.pass.cpp (+174)
(added) libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left_with_iter/valid.pass.cpp (+116)
(added) libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/gaussian_sum.h (+19)
(added) libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/requirements.h (+82)
(modified) libcxx/test/std/iterators/iterator.requirements/iterator.concepts/iterator.concept.input/input_iterator.compile.pass.cpp (+1)
(modified) libcxx/test/std/library/description/conventions/customization.point.object/niebloid.compile.pass.cpp (+4)
(modified) libcxx/test/std/ranges/range.adaptors/range.all/range.ref.view/borrowing.compile.pass.cpp (+1-1)
(modified) libcxx/test/support/test_range.h (+28-26)

diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index d8faf6467b79a..95702288a3c2f 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -22,6 +22,7 @@ set(files
   __algorithm/find_first_of.h
   __algorithm/find_if.h
   __algorithm/find_if_not.h
+  __algorithm/fold.h
   __algorithm/for_each.h
   __algorithm/for_each_n.h
   __algorithm/for_each_segment.h
diff --git a/libcxx/include/__algorithm/fold.h b/libcxx/include/__algorithm/fold.h
new file mode 100644
index 0000000000000..ad4e9820b479f
--- /dev/null
+++ b/libcxx/include/__algorithm/fold.h
@@ -0,0 +1,118 @@
+// -*- 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 {
+  _Ip in;
+  _Tp result;
+};
+
+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 constexpr auto
+  operator()(_Ip __first, _Sp __last, _Tp __init, _Fp __f) const {
+    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 constexpr auto operator()(_Rp&& __r, _Tp __init, _Fp __f) const {
+    auto __result = (*this)(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.result)};
+  }
+};
+
+inline namespace __cpo {
+inline constexpr auto fold_left_with_iter = __fold_left_with_iter();
+} // namespace __cpo
+
+struct __fold_left {
+  template <input_iterator _Ip, sentinel_for<_Ip> _Sp, class _Tp, __indirectly_binary_left_foldable<_Tp, _Ip> _Fp>
+  _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr auto
+  operator()(_Ip __first, _Sp __last, _Tp __init, _Fp __f) const {
+    return fold_left_with_iter(std::move(__first), std::move(__last), std::move(__init), std::ref(__f)).result;
+  }
+
+  template <input_range _Rp, class _Tp, __indirectly_binary_left_foldable<_Tp, iterator_t<_Rp>> _Fp>
+  _LIBCPP_NODISCARD_EXT _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Rp&& __r, _Tp __init, _Fp __f) const {
+    return fold_left_with_iter(ranges::begin(__r), ranges::end(__r), std::move(__init), std::ref(__f)).result;
+  }
+};
+
+inline namespace __cpo {
+inline constexpr auto fold_left = __fold_left();
+} // namespace __cpo
+} // namespace ranges
+
+#endif // _LIBCPP_STD_VER >= 23
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___ALGORITHM_FOLD_H
diff --git a/libcxx/include/algorithm b/libcxx/include/algorithm
index 627e7d20213fe..3fd9ba10f6310 100644
--- a/libcxx/include/algorithm
+++ b/libcxx/include/algorithm
@@ -1778,6 +1778,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>
diff --git a/libcxx/include/module.modulemap.in b/libcxx/include/module.modulemap.in
index 90ee7fbb2157c..359de41fdd128 100644
--- a/libcxx/include/module.modulemap.in
+++ b/libcxx/include/module.modulemap.in
@@ -665,6 +665,7 @@ module std_private_algorithm_find_end                                    [system
 module std_private_algorithm_find_first_of                               [system] { header "__algorithm/find_first_of.h" }
 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_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" }
diff --git a/libcxx/test/libcxx/diagnostics/ranges.nodiscard_extensions.verify.cpp b/libcxx/test/libcxx/diagnostics/ranges.nodiscard_extensions.verify.cpp
index d11dcfcd0d2e4..f0a0e4889a760 100644
--- a/libcxx/test/libcxx/diagnostics/ranges.nodiscard_extensions.verify.cpp
+++ b/libcxx/test/libcxx/diagnostics/ranges.nodiscard_extensions.verify.cpp
@@ -12,6 +12,8 @@
 
 #include <algorithm>
 
+#include "test_macros.h"
+
 void test() {
   int range[1];
   int* iter = range;
@@ -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
 }
diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/requirements.compile.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/requirements.compile.pass.cpp
new file mode 100644
index 0000000000000..0b1c1e4eae1ea
--- /dev/null
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/requirements.compile.pass.cpp
@@ -0,0 +1,99 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// <algorithm>
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+
+// Checks that `std::ranges::fold_left`'s requirements are correct.
+
+#include <algorithm>
+#include <concepts>
+#include <functional>
+#include <iterator>
+#include <ranges>
+
+#include "test_iterators.h"
+#include "../requirements.h"
+
+// Covers indirectly_readable<I> too
+template <std::input_or_output_iterator T>
+  requires(!std::input_iterator<T>)
+void requires_input_iterator() {
+  static_assert(!requires(T t) { std::ranges::fold_left(t, std::unreachable_sentinel, 0, std::plus()); });
+}
+
+template <std::equality_comparable T>
+  requires(!std::sentinel_for<int*, T>)
+void requires_sentinel() {
+  static_assert(!requires(T first, T last) { std::ranges::fold_left(first, last, 0, std::plus()); });
+}
+
+template <class F>
+  requires(!std::copy_constructible<F>)
+void requires_copy_constructible_F() {
+  static_assert(!requires(int* first, int* last, F f) { std::ranges::fold_left(first, last, 0, std::move(f)); });
+}
+
+template <class F>
+  requires(!std::invocable<F&, int, std::iter_reference_t<int*>>)
+void requires_raw_invocable() {
+  static_assert(!requires(int* first, int* last, F f) { std::ranges::fold_left(first, last, 0, f); });
+}
+
+template <class F>
+  requires(!std::convertible_to<std::invoke_result_t<F&, S, std::iter_reference_t<S*>>,
+                                std::decay_t<std::invoke_result_t<F&, S, std::iter_reference_t<S*>>>>)
+void requires_decaying_invoke_result() {
+  static_assert(!requires(S* first, S* last, S init, F f) { std::ranges::fold_left(first, last, init, f); });
+}
+
+template <class T>
+  requires(!std::movable<T>)
+void requires_movable_init() {
+  static_assert(!requires(copyable_non_movable* first, copyable_non_movable* last, T init) {
+    std::ranges::fold_left(first, last, init, std::plus());
+  });
+}
+
+template <class T>
+  requires(!std::movable<T>)
+void requires_movable_decayed() {
+  static_assert(!requires(T* first, T* last) { std::ranges::fold_left(first, last, 0, std::minus()); });
+}
+
+template <class T>
+  requires(!std::convertible_to<T, int>)
+void requires_init_is_convertible_to_decayed() {
+  static_assert(!requires(int* first, int* last, T init) { std::ranges::fold_left(first, last, init, std::plus()); });
+}
+
+template <class T>
+  requires(!std::invocable<std::plus<>&, T, T&>)
+void requires_invocable_with_decayed() {
+  static_assert(!requires(T* first, T* last, int init) { std::ranges::fold_left(first, last, init, std::plus()); });
+}
+
+template <class T>
+  requires(!std::assignable_from<T&, T volatile&>)
+void requires_assignable_from_invoke_result() {
+  static_assert(!requires(T* first, T* last, T init) { std::ranges::fold_left(first, last, init, std::plus()); });
+}
+
+void test() {
+  requires_input_iterator<bad_iterator_category>();
+  requires_sentinel<cpp17_input_iterator<int*>>();
+  requires_copy_constructible_F<non_copy_constructible_callable>();
+  requires_raw_invocable<not_invocable>();
+  requires_decaying_invoke_result<non_decayable_result>();
+  requires_movable_init<non_movable>();
+  requires_movable_decayed<copyable_non_movable>();
+  requires_init_is_convertible_to_decayed<not_convertible_to_int>();
+  requires_invocable_with_decayed<not_invocable_with_decayed>();
+  requires_assignable_from_invoke_result<not_assignable_to_decayed>();
+}
diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/return_types.compile.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/return_types.compile.pass.cpp
new file mode 100644
index 0000000000000..911ce1615a1c7
--- /dev/null
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/return_types.compile.pass.cpp
@@ -0,0 +1,171 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// <algorithm>
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+
+// Checks that `std::ranges::fold_left`'s return type is correct.
+
+#include <algorithm>
+#include <concepts>
+#include <functional>
+#include <ranges>
+
+#include "test_iterators.h"
+#include "test_range.h"
+
+template <class Result, class T>
+concept is_T = std::same_as<Result, T>;
+
+using std::ranges::fold_left;
+[[maybe_unused]] auto f = [](int x, double y) { return x * y; };
+
+struct Int {
+  int value;
+};
+
+struct Long {
+  int value;
+
+  Long plus(Int) const;
+};
+
+namespace sentinel_based_ranges {
+template <class T>
+using cpp17_input_range = test_range<cpp17_input_iterator, T>;
+
+static_assert(requires(cpp17_input_range<int> r) {
+  { fold_left(r.begin(), r.end(), 0, std::plus()) } -> std::same_as<int>;
+});
+static_assert(requires(cpp17_input_range<int> r) {
+  { fold_left(r, 0, std::plus()) } -> std::same_as<int>;
+});
+static_assert(requires(cpp17_input_range<int> r) {
+  { fold_left(std::move(r), 0, std::plus()) } -> std::same_as<int>;
+});
+
+template <class T>
+using cpp20_input_range = test_range<cpp20_input_iterator, T>;
+
+static_assert(requires(cpp20_input_range<int> r) {
+  { fold_left(r.begin(), r.end(), 0, std::plus()) } -> std::same_as<int>;
+});
+static_assert(requires(cpp20_input_range<int> r) {
+  { fold_left(r, 0, std::plus()) } -> std::same_as<int>;
+});
+static_assert(requires(cpp20_input_range<int> r) {
+  { fold_left(std::move(r), 0, std::plus()) } -> std::same_as<int>;
+});
+
+template <class T>
+using forward_range = test_range<forward_iterator, T>;
+
+static_assert(requires(forward_range<int> r) {
+  { fold_left(r.begin(), r.end(), 0, std::plus()) } -> std::same_as<int>;
+});
+static_assert(requires(forward_range<int> r) {
+  { fold_left(r, 0, std::plus()) } -> std::same_as<int>;
+});
+static_assert(requires(forward_range<int> r) {
+  { fold_left(std::move(r), 0, std::plus()) } -> std::same_as<int>;
+});
+
+template <class T>
+using bidirectional_range = test_range<bidirectional_iterator, T>;
+
+static_assert(requires(bidirectional_range<short> r) {
+  { fold_left(r.begin(), r.end(), 0, f) } -> std::same_as<double>;
+});
+static_assert(requires(bidirectional_range<short> r) {
+  { fold_left(r, 0, f) } -> std::same_as<double>;
+});
+static_assert(requires(bidirectional_range<short> r) {
+  { fold_left(std::move(r), 0, f) } -> std::same_as<double>;
+});
+
+template <class T>
+using random_access_range = test_range<random_access_iterator, T>;
+
+static_assert(requires(random_access_range<int> r) {
+  { fold_left(r.begin(), r.end(), 0, f) } -> std::same_as<double>;
+});
+static_assert(requires(random_access_range<int> r) {
+  { fold_left(r, 0, f) } -> std::same_as<double>;
+});
+static_assert(requires(random_access_range<int> r) {
+  { fold_left(std::move(r), 0, f) } -> std::same_as<double>;
+});
+
+template <class T>
+using contiguous_range = test_range<contiguous_iterator, T>;
+
+static_assert(requires(contiguous_range<int> r) {
+  { fold_left(r.begin(), r.end(), 0.0f, std::plus()) } -> std::same_as<float>;
+});
+static_assert(requires(contiguous_range<int> r) {
+  { fold_left(r, 0.0f, std::plus()) } -> std::same_as<float>;
+});
+static_assert(requires(contiguous_range<int> r) {
+  { fold_left(std::move(r), 0.0f, std::plus()) } -> std::same_as<float>;
+});
+} // namespace sentinel_based_ranges
+
+namespace common_ranges {
+template <class T>
+using forward_range = test_common_range<forward_iterator, T>;
+
+static_assert(requires(forward_range<Int> r) {
+  { fold_left(r.begin(), r.end(), Long(0), &Long::plus) } -> std::same_as<Long>;
+});
+static_assert(requires(forward_range<Int> r) {
+  { fold_left(r, Long(0), &Long::plus) } -> std::same_as<Long>;
+});
+static_assert(requires(forward_range<Int> r) {
+  { fold_left(std::move(r), Long(0), &Long::plus) } -> std::same_as<Long>;
+});
+
+template <class T>
+using bidirectional_range = test_common_range<bidirectional_iterator, T>;
+
+static_assert(requires(bidirectional_range<int> r) {
+  { fold_left(r.begin(), r.end(), 0, std::plus()) } -> std::same_as<int>;
+});
+static_assert(requires(bidirectional_range<int> r) {
+  { fold_left(r, 0, std::plus()) } -> std::same_as<int>;
+});
+static_assert(requires(bidirectional_range<int> r) {
+  { fold_left(std::move(r), 0, std::plus()) } -> std::same_as<int>;
+});
+
+template <class T>
+using random_access_range = test_common_range<random_access_iterator, T>;
+
+static_assert(requires(random_access_range<int> r) {
+  { fold_left(r.begin(), r.end(), 0, f) } -> std::same_as<double>;
+});
+static_assert(requires(random_access_range<int> r) {
+  { fold_left(r, 0, f) } -> std::same_as<double>;
+});
+static_assert(requires(random_access_range<int> r) {
+  { fold_left(std::move(r), 0, f) } -> std::same_as<double>;
+});
+
+template <class T>
+using contiguous_range = test_common_range<contiguous_iterator, T>;
+
+static_assert(requires(contiguous_range<int> r) {
+  { fold_left(r.begin(), r.end(), 0.0f, std::plus()) } -> std::same_as<float>;
+});
+static_assert(requires(contiguous_range<int> r) {
+  { fold_left(r, 0.0f, std::plus()) } -> std::same_as<float>;
+});
+static_assert(requires(contiguous_range<int> r) {
+  { fold_left(std::move(r), 0.0f, std::plus()) } -> std::same_as<float>;
+});
+} // namespace common_ranges
diff --git a/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/valid.pass.cpp b/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/valid.pass.cpp
new file mode 100644
index 0000000000000..ef0fbb4b2a6d3
--- /dev/null
+++ b/libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/valid.pass.cpp
@@ -0,0 +1,93 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// <algorithm>
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+
+// template<input_iterator I, sentinel_for<I> S, class T,
+//          indirectly-binary-left-foldable<T, I> F>
+//   constexpr see below ranges::fold_left(I first, S last, T init, F f);
+//
+// template<input_range R, class T, indirectly-binary-left-foldable<T, iterator_t<R>> F>
+//   constexpr see below ranges::fold_left(R&& r, T init, F f);
+
+#include <algorithm>
+#include <cassert>
+#include <vector>
+#include <functional>
+
+#include "test_range.h"
+#include "../gaussian_sum.h"
+
+constexpr bool test() {
+  {
+    auto data         = std::vector<int>{1, 2, 3, 4};
+    auto const result = std::ranges::fold_left(data.begin(), data.begin(), 0, std::plus());
+
+    assert(result == 0);
+
+    auto range = std::span(data.data(), 0);
+    assert(std::ranges::fold_left(range, 0, std::plus()) == result);
+  }
+  {
+    auto data           = std::vector<int>{1, 2, 3, 4};
+    constexpr auto init = 100.1;
+    auto const result   = std::ranges::fold_left(data.begin(), data.begin(), init, std::plus());
+
+    assert(result ==...
[truncated]

libcxx/include/__algorithm/fold.h

EricWF

I hear an updated change is coming, so I'm submitting my comments.

libcxx/include/__algorithm/fold.h

libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left_with_iter/valid.pass.cpp

libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/valid.pass.cpp

EricWF

My only blocking concern is the clang verify tests. They're too brittle and will break.
Otherwise, This change LGTM and is ready to land afterwards.

libcxx/include/__algorithm/fold.h

libcxx/test/libcxx/diagnostics/ranges.nodiscard_extensions.verify.cpp

libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/requirements.verify.cpp

libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left_with_iter/valid.pass.cpp

libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/gaussian_sum.h

libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/requirements.h

tcanens · 2023-12-15T01:43:06Z

libcxx/include/__algorithm/fold.h

+  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) {
+    return fold_left_with_iter(std::move(__first), std::move(__last), std::move(__init), std::ref(__f)).result;


There are two moves here (one in fold_left_with_iter to pack __result into the returned struct, and another one here to unpack it that could in principle be elided due to the Returns: element. (Eliding both would depend on NRVO kicking in.)

EricWF

My only blocking concern is the clang verify tests. They're too brittle and will break. Otherwise, This change LGTM and is ready to land afterwards.

OK, so Chris and I discussed it, and we want to resolve this by reverting to these type of tests:
bec1572

So LGTM once that has been addressed! Let's land this!

This reverts commit bec1572.

This reverts commit 6a57523.

This reverts commit 12f36c4.

var-const

Thanks a lot for working on this!

libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/gaussian_sum.h

libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/requirements.verify.cpp

var-const · 2023-12-15T19:54:56Z

libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/requirements.verify.cpp

+// expected-error@*:* 19 {{no matching function for call to object of type 'const __fold_left'}}
+
+void test_iterator() {
+  // expected-note@*:*  10 {{candidate template ignored: constraints not satisfied}}


I understand the desire to make sure we get the exact error we expect, but it seems a little overspecified. Can we discuss this on Discord with the broader group?

Per a video call with @EricWF, I'm going to revert bec1572 and the two commits that come after it, which will probably address what you and @philnik777 have expressed concerns over.

...xx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left_with_iter/requirements.verify.cpp

libcxx/include/algorithm

...est/std/library/description/conventions/customization.point.object/niebloid.compile.pass.cpp

@EricWF

Per @EricWF's feedback, I've consolidated all the requirements tests into a single file in order to eliminate duplication among the various fold algorithms (which have similar requirements).

libcxx/include/__algorithm/fold.h

var-const

Finished the first pass. This looks pretty good, just a few comments.

libcxx/include/algorithm

var-const · 2023-12-15T20:05:18Z

libcxx/include/__algorithm/fold.h

+} // namespace __cpo
+
+struct __fold_left {
+  template <input_iterator _Ip, sentinel_for<_Ip> _Sp, class _Tp, __indirectly_binary_left_foldable<_Tp, _Ip> _Fp>


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

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.

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

@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.

@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.

@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.

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.

See #76540 for the renaming patch.

libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/valid.pass.cpp

libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left_with_iter/valid.pass.cpp

libcxx/test/libcxx/diagnostics/ranges.nodiscard_extensions.verify.cpp

libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/requirements.verify.cpp

libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/fold_left/return_types.compile.pass.cpp

libcxx/include/__algorithm/fold.h

libcxx/test/support/invocable_with_telemetry.h

EricWF

LGTM again. Thanks for spending so much time on on these tests.

libcxx/test/support/invocable_with_telemetry.h

libcxx/test/support/maths.h

github-actions · 2023-12-19T01:32:39Z

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

… to avoid comments

EricWF

LGTM. I see @var-const has "requested changes" set, but I feel we've addressed the one unresolved comment of his.

Given the amount of work you're going to be doing in the new year adding more fold algorithms, this code is going to get a lot of love still yet.

Any concerns that haven't been satisfactorily addressed will certainly be addressed in those reviews, if not earlier.

I would like for this change to land, before we all have such good holidays that we forget all about it.

@cjdb Without further input from @var-const , I believe you should commit this change. As long as your willing to work diligently to resolve any post-commit feedback that occurs.

EricWF · 2023-12-20T00:05:05Z

libcxx/include/__algorithm/fold.h

+} // namespace __cpo
+
+struct __fold_left {
+  template <input_iterator _Ip, sentinel_for<_Ip> _Sp, class _Tp, __indirectly_binary_left_foldable<_Tp, _Ip> _Fp>


@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.

cjdb · 2023-12-20T05:57:18Z

Merging, with the promise to diligently resolve post-commit feedback in early January. Anything that's specific to this patch will be actioned before starting work on the remaining fold algorithms, and anything that's generalised to std::ranges will be actioned after finishing them.

var-const

This patch has changed substantially since I last reviewed it. I'll send more feedback in a follow-up review.

var-const · 2023-12-19T20:13:42Z

libcxx/include/__algorithm/fold.h

+} // namespace __cpo
+
+struct __fold_left {
+  template <input_iterator _Ip, sentinel_for<_Ip> _Sp, class _Tp, __indirectly_binary_left_foldable<_Tp, _Ip> _Fp>


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

var-const · 2023-12-19T20:15:54Z

libcxx/test/std/algorithms/algorithms.results/in_value_result.pass.cpp

+static_assert(std::is_move_constructible_v<std::ranges::in_value_result<MoveOnly, int>>);
+static_assert(std::is_move_constructible_v<std::ranges::in_value_result<int, MoveOnly>>);
+
+// should not copy constructible with move-only type


Ultranit: s/should not copy constructible/should not be copy constructible/.

I fully agree that adding this to a project-wide style guide would be very useful (to be clear, I don't mean to imply it has to be a comprehensive huge guide, even if it documents just a few randomly chosen aspects, it's already helpful and we can always expand later). We do have a "Coding standards" section in the Contributing document -- we might want to add a new page and link it from that section. Thanks for taking this on!

I fully agree that adding this to a project-wide style guide would be very useful (to be clear, I don't mean to imply it has to be a comprehensive huge guide, even if it documents just a few randomly chosen aspects, it's already helpful and we can always expand later). We do have a "Coding standards" section in the Contributing document -- we might want to add a new page and link it from that section. Thanks for taking this on!

Re ultranit: applied in #76534.

Re second comment: Not sure what this is pertaining to?

libcxx/test/std/algorithms/algorithms.results/in_value_result.pass.cpp

var-const · 2023-12-19T20:22:20Z

libcxx/test/support/invocable_with_telemetry.h

+#if TEST_STD_VER < 20
+#  error invocable_with_telemetry requires C++20
+#else
+struct invocable_telemetry {


Is there precedent for calling this kind of measurements "telemetry"? I've only ever seen "telemetry" to describe a somewhat higher-level and heavier-weight kind of measurements about the behavior of the application in general, usually involving transmitting data over a network (which seems to be implied by the tele part).

Thoughts summarised in the comment below.

var-const · 2023-12-19T20:26:57Z

libcxx/test/support/invocable_with_telemetry.h

+//
+//===----------------------------------------------------------------------===//
+
+#ifndef TEST_SUPPORT_INVOCABLE_WITH_TELEMETRY_H


No action required: we already have at least TracedCopyMove and TracedAssignment in support/copy_move_types.h, and I'm almost confident there are a few other similar types defined more locally in various tests. Is this class intended to supersede those? Or to provide C++20-only benefits?

My guess is "probably not", though we should come up with a consistent design (happy to work with you on that). I wasn't aware of those two, but the contents of support/counting_predicates.h weren't quite right for the application. I suggest we consolidate all of the counting invocable types into a single counting_invocable or TracedInvocable so that there's only one canonical type.

Looking at TracedCopyMove, it looks like it'll need a call operator added. Would you like me to add tests for explicitly counting copies and moves? It may be better to have these be separate tests for modularity.

Nothing has been applied yet, but I suspect a change to be made in the next week or so.

libcxx/test/std/algorithms/algorithms.results/in_value_result.pass.cpp

libcxx/test/std/algorithms/alg.nonmodifying/alg.fold/requirements.compile.pass.cpp

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

var-const · 2023-12-20T19:04:15Z

libcxx/include/__algorithm/fold.h

+} // namespace __cpo
+
+struct __fold_left {
+  template <input_iterator _Ip, sentinel_for<_Ip> _Sp, class _Tp, __indirectly_binary_left_foldable<_Tp, _Ip> _Fp>


@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.

var-const · 2023-12-21T01:19:52Z

I see @var-const has "requested changes" set, but I feel we've addressed the one unresolved comment of his.

@EricWF In fact, this patch changed quite significantly since I last looked at it (based on your feedback, I believe), so I would have appreciated being able to do another round of review before this was merged.

Some of the feedback was also relevant to other files, and has been applied there too.

cjdb · 2023-12-28T21:25:21Z

I see @var-const has "requested changes" set, but I feel we've addressed the one unresolved comment of his.

@EricWF In fact, this patch changed quite significantly since I last looked at it (based on your feedback, I believe), so I would have appreciated being able to do another round of review before this was merged.

That is fair. My understanding is that Eric was trying to balance respecting that you might have feedback with me struggling to switch off while OOO, if a PR is still in my "active context".

I ended up coming back into the office early, and prioritised applying your feedback as a result (see #76534). Please do make further commentary as necessary.

…6534) Some of the feedback was also relevant to other files, and has been applied there too.

cjdb added 2 commits December 11, 2023 18:44

[libcxx] extends test range types to take a value-type

e36a06d

This increases the types' versatility so that they're not restricted just to `int*`.

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

b373740

cjdb added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 13, 2023

cjdb requested review from lnihlen, var-const and philnik777 December 13, 2023 00:19

cjdb requested a review from a team as a code owner December 13, 2023 00:19

var-const self-assigned this Dec 13, 2023

changes to using static opertaor()

e776610

hawkinsw reviewed Dec 13, 2023

View reviewed changes

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

cjdb added 2 commits December 14, 2023 19:14

adds missing export

15b686b

changes requirements test from asserts to verify

bec1572

hawkinsw reviewed Dec 14, 2023

View reviewed changes

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

cjdb added 2 commits December 14, 2023 22:40

adds missing file

6a57523

repeats for fold_left

12f36c4

EricWF reviewed Dec 14, 2023

View reviewed changes

EricWF requested changes Dec 15, 2023

View reviewed changes

tcanens reviewed Dec 15, 2023

View reviewed changes

EricWF approved these changes Dec 15, 2023

View reviewed changes

cjdb added 3 commits December 15, 2023 19:42

Revert "changes requirements test from asserts to verify"

07ddbb4

This reverts commit bec1572.

Revert "adds missing file"

5376bec

This reverts commit 6a57523.

Revert "repeats for fold_left"

c806db0

This reverts commit 12f36c4.

var-const requested changes Dec 15, 2023

View reviewed changes

cjdb added 2 commits December 15, 2023 21:38

consolidates requirements.compile.pass.cpp for the fold algos

5523376

Per @EricWF's feedback, I've consolidated all the requirements tests into a single file in order to eliminate duplication among the various fold algorithms (which have similar requirements).

eliminates unnecessary __cpo namespace

5822e9d

cjdb commented Dec 15, 2023

View reviewed changes

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

var-const requested changes Dec 15, 2023

View reviewed changes

philnik777 reviewed Dec 15, 2023

View reviewed changes

reworks valid.cpp to incorporate feedback

2e20461

cjdb added 2 commits December 18, 2023 22:47

adds documentation and fixes telemetry move

b0e5f7c

fixes telemetry move assignment

824e1fe

EricWF reviewed Dec 18, 2023

View reviewed changes

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

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

libcxx/test/support/invocable_with_telemetry.h Show resolved Hide resolved

libcxx/test/support/invocable_with_telemetry.h Outdated Show resolved Hide resolved

EricWF approved these changes Dec 18, 2023

View reviewed changes

cjdb added 3 commits December 19, 2023 00:25

fixes in_value_result

1b1b3d4

replaces rogue integers with a telemetry struct

141be97

Merge branch 'main' into fold_left

bdd4f59

cjdb added 8 commits December 19, 2023 01:47

applies clang-format

70e7764

adds _LIBCPP_HIDE_FROM_ABI to in_value_result

c8aa9e6

manually fixes clang-format issue

33bc982

handles cxx26 action failure

f68da2c

fixes CI for C++20

15cc627

adds tests for pointer-to-member functions and renames test functions…

80bc73b

… to avoid comments

changes parameter name so it doesn't shadow

991db00

handle platforms that don't support certain features

7bc4409

EricWF approved these changes Dec 20, 2023

View reviewed changes

handle platforms that don't support certain features

2b44b37

cjdb merged commit 3903438 into llvm:main Dec 20, 2023

var-const reviewed Dec 20, 2023

View reviewed changes

cjdb added a commit to cjdb/llvm-project that referenced this pull request Dec 28, 2023

[libcxx] applies changes regarding post-commit feedback to llvm#75259

d4ab164

Some of the feedback was also relevant to other files, and has been applied there too.

cjdb added a commit that referenced this pull request Apr 11, 2024

[libcxx] applies changes regarding post-commit feedback to #75259 (#7…

f0ea888

…6534) Some of the feedback was also relevant to other files, and has been applied there too.

vimpostor mentioned this pull request Jun 9, 2024

Build fails on macOS vimpostor/blobdrop#7

Closed

JCGoran mentioned this pull request Jan 3, 2025

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

Open

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

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

Uh oh!

Conversation

cjdb commented Dec 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Dec 13, 2023

Uh oh!

Uh oh!

Uh oh!

EricWF left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

EricWF left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EricWF left a comment

Choose a reason for hiding this comment

Uh oh!

var-const left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

var-const left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cjdb commented Dec 13, 2023 •

edited

Loading

github-actions bot commented Dec 19, 2023 •

edited

Loading