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.

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] P2387R3: Pipe support for user-defined range adaptors #89148

Merged
merged 5 commits into from
Apr 23, 2024

Conversation

xiaoyang-sde
Copy link
Member

@xiaoyang-sde xiaoyang-sde commented Apr 17, 2024

Abstract

This pull request aims to finalize the std::ranges::range_adaptor_closure class template from P2387R3.

// [range.adaptor.object], range adaptor objects
template<class D>
  requires is_class_v<D> && same_as<D, remove_cv_t<D>>
class range_adaptor_closure { };

Implementation

The current implementation of __range_adaptor_closure was introduced in ee44dd8 and has served as the foundation for the range adaptors in libc++ for a while. This pull request keeps its implementation, with the exception of the following changes:

  • __range_adaptor_closure now includes the missing constraints is_class_v<D> && same_as<D, remove_cv_t<D>> to restrict the type of class that can inherit from it. ([ranges.syn])
  • The operator| of __range_adaptor_closure no longer requires its first argument to model viewable_range. ([range.adaptor.object]/1)
  • The _RangeAdaptorClosure concept is refined to exclude cases where T models range or where T has base classes of type range_adaptor_closure<U> for another type U. ([range.adaptor.object]/2)

Reference

@xiaoyang-sde xiaoyang-sde requested a review from a team as a code owner April 17, 2024 21:41
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Apr 17, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 17, 2024

@llvm/pr-subscribers-libcxx

Author: Xiaoyang Liu (xiaoyang-sde)

Changes

Abstract

This pull request aims to finalize the std::ranges:: range_adaptor_closure class template from P2387R3.

// [range.adaptor.object], range adaptor objects
template&lt;class D&gt;
  requires is_class_v&lt;D&gt; &amp;&amp; same_as&lt;D, remove_cv_t&lt;D&gt;&gt;
class range_adaptor_closure { };

Implementation

The current implementation of __range_adaptor_closure was introduced in ee44dd8 and has served as the foundation for the range adaptors in libc++ for a while. This pull request keeps its implementation, with the exception of the following changes:

  • __range_adaptor_closure now includes the missing constraints is_class_v&lt;D&gt; &amp;&amp; same_as&lt;D, remove_cv_t&lt;D&gt;&gt; to restrict the type of class that can inherit from it. ([ranges.syn])
  • The operator| of __range_adaptor_closure no longer requires its first argument to model viewable_range. ([[range.adaptor.object]/1](https://eel.is/c++draft/range.adaptor.object#1))
  • The _RangeAdaptorClosure concept is refined to exclude cases where T models range or where T has base classes of type range_adaptor_closure&lt;U&gt; for another type U. ([[range.adaptor.object]/2](https://eel.is/c++draft/range.adaptor.object#2))

Reference


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

6 Files Affected:

  • (modified) libcxx/docs/ReleaseNotes/19.rst (+1)
  • (modified) libcxx/docs/Status/Cxx23.rst (-1)
  • (modified) libcxx/docs/Status/Cxx23Papers.csv (+1-1)
  • (modified) libcxx/include/__ranges/range_adaptor.h (+20-5)
  • (modified) libcxx/include/ranges (+5)
  • (added) libcxx/test/std/ranges/range.adaptors/range.adaptor.object/range_adaptor_closure.pass.cpp (+128)
diff --git a/libcxx/docs/ReleaseNotes/19.rst b/libcxx/docs/ReleaseNotes/19.rst
index 53cc7a77d1af48..bccbe2f10d13f0 100644
--- a/libcxx/docs/ReleaseNotes/19.rst
+++ b/libcxx/docs/ReleaseNotes/19.rst
@@ -49,6 +49,7 @@ Implemented Papers
 - P2302R4 - ``std::ranges::contains``
 - P1659R3 - ``std::ranges::starts_with`` and ``std::ranges::ends_with``
 - P3029R1 - Better ``mdspan``'s CTAD
+- P2387R3 - Pipe support for user-defined range adaptors
 
 Improvements and New Features
 -----------------------------
diff --git a/libcxx/docs/Status/Cxx23.rst b/libcxx/docs/Status/Cxx23.rst
index b19ff4fdc0f79e..23d30c8128d71e 100644
--- a/libcxx/docs/Status/Cxx23.rst
+++ b/libcxx/docs/Status/Cxx23.rst
@@ -43,7 +43,6 @@ Paper Status
    .. [#note-P0533R9] P0533R9: ``isfinite``, ``isinf``, ``isnan`` and ``isnormal`` are implemented.
    .. [#note-P1413R3] P1413R3: ``std::aligned_storage_t`` and ``std::aligned_union_t`` are marked deprecated, but
       clang doesn't issue a diagnostic for deprecated using template declarations.
-   .. [#note-P2387R3] P2387R3: ``bind_back`` only
    .. [#note-P2520R0] P2520R0: Libc++ implemented this paper as a DR in C++20 as well.
    .. [#note-P2711R1] P2711R1: ``join_with_view`` hasn't been done yet since this type isn't implemented yet.
    .. [#note-P2770R0] P2770R0: ``join_with_view`` hasn't been done yet since this type isn't implemented yet.
diff --git a/libcxx/docs/Status/Cxx23Papers.csv b/libcxx/docs/Status/Cxx23Papers.csv
index 065db97a0b0b15..f75dd288304b27 100644
--- a/libcxx/docs/Status/Cxx23Papers.csv
+++ b/libcxx/docs/Status/Cxx23Papers.csv
@@ -45,7 +45,7 @@
 "`P1413R3 <https://wg21.link/P1413R3>`__","LWG","Deprecate ``std::aligned_storage`` and ``std::aligned_union``","February 2022","|Complete| [#note-P1413R3]_",""
 "`P2255R2 <https://wg21.link/P2255R2>`__","LWG","A type trait to detect reference binding to temporary","February 2022","",""
 "`P2273R3 <https://wg21.link/P2273R3>`__","LWG","Making ``std::unique_ptr`` constexpr","February 2022","|Complete|","16.0"
-"`P2387R3 <https://wg21.link/P2387R3>`__","LWG","Pipe support for user-defined range adaptors","February 2022","|Partial| [#note-P2387R3]_","","|ranges|"
+"`P2387R3 <https://wg21.link/P2387R3>`__","LWG","Pipe support for user-defined range adaptors","February 2022","|Complete|","19.0","|ranges|"
 "`P2440R1 <https://wg21.link/P2440R1>`__","LWG","``ranges::iota``, ``ranges::shift_left`` and ``ranges::shift_right``","February 2022","","","|ranges|"
 "`P2441R2 <https://wg21.link/P2441R2>`__","LWG","``views::join_with``","February 2022","|In Progress|","","|ranges|"
 "`P2442R1 <https://wg21.link/P2442R1>`__","LWG","Windowing range adaptors: ``views::chunk`` and ``views::slide``","February 2022","","","|ranges|"
diff --git a/libcxx/include/__ranges/range_adaptor.h b/libcxx/include/__ranges/range_adaptor.h
index 726b7eda019ee3..a8b33ac620c4d2 100644
--- a/libcxx/include/__ranges/range_adaptor.h
+++ b/libcxx/include/__ranges/range_adaptor.h
@@ -19,6 +19,7 @@
 #include <__functional/invoke.h>
 #include <__ranges/concepts.h>
 #include <__type_traits/decay.h>
+#include <__type_traits/is_class.h>
 #include <__type_traits/is_nothrow_constructible.h>
 #include <__type_traits/remove_cvref.h>
 #include <__utility/forward.h>
@@ -41,6 +42,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 // - `x | f` is equivalent to `f(x)`
 // - `f1 | f2` is an adaptor closure `g` such that `g(x)` is equivalent to `f2(f1(x))`
 template <class _Tp>
+  requires is_class_v<_Tp> && same_as<_Tp, remove_cv_t<_Tp>>
 struct __range_adaptor_closure;
 
 // Type that wraps an arbitrary function object and makes it into a range adaptor closure,
@@ -52,15 +54,21 @@ struct __range_adaptor_closure_t : _Fn, __range_adaptor_closure<__range_adaptor_
 _LIBCPP_CTAD_SUPPORTED_FOR_TYPE(__range_adaptor_closure_t);
 
 template <class _Tp>
-concept _RangeAdaptorClosure = derived_from<remove_cvref_t<_Tp>, __range_adaptor_closure<remove_cvref_t<_Tp>>>;
+_Tp __derived_from_range_adaptor_closure(__range_adaptor_closure<_Tp>*);
 
 template <class _Tp>
+concept _RangeAdaptorClosure = !ranges::range<remove_cvref_t<_Tp>> && requires {
+  { __derived_from_range_adaptor_closure((remove_cvref_t<_Tp>*)nullptr) } -> same_as<remove_cvref_t<_Tp>>;
+};
+
+template <class _Tp>
+  requires is_class_v<_Tp> && same_as<_Tp, remove_cv_t<_Tp>>
 struct __range_adaptor_closure {
-  template <ranges::viewable_range _View, _RangeAdaptorClosure _Closure>
-    requires same_as<_Tp, remove_cvref_t<_Closure>> && invocable<_Closure, _View>
+  template <ranges::range _Range, _RangeAdaptorClosure _Closure>
+    requires same_as<_Tp, remove_cvref_t<_Closure>> && invocable<_Closure, _Range>
   [[nodiscard]] _LIBCPP_HIDE_FROM_ABI friend constexpr decltype(auto)
-  operator|(_View&& __view, _Closure&& __closure) noexcept(is_nothrow_invocable_v<_Closure, _View>) {
-    return std::invoke(std::forward<_Closure>(__closure), std::forward<_View>(__view));
+  operator|(_Range&& __range, _Closure&& __closure) noexcept(is_nothrow_invocable_v<_Closure, _Range>) {
+    return std::invoke(std::forward<_Closure>(__closure), std::forward<_Range>(__range));
   }
 
   template <_RangeAdaptorClosure _Closure, _RangeAdaptorClosure _OtherClosure>
@@ -73,6 +81,13 @@ struct __range_adaptor_closure {
   }
 };
 
+#  if _LIBCPP_STD_VER >= 23
+namespace ranges {
+template <class _Tp>
+using range_adaptor_closure = __range_adaptor_closure<_Tp>;
+} // namespace ranges
+#  endif // _LIBCPP_STD_VER >= 23
+
 #endif // _LIBCPP_STD_VER >= 20
 
 _LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/ranges b/libcxx/include/ranges
index 167d2137eaf454..07a525ed8641fd 100644
--- a/libcxx/include/ranges
+++ b/libcxx/include/ranges
@@ -93,6 +93,11 @@ namespace std::ranges {
   template<class T>
   concept viewable_range = see below;
 
+  // [range.adaptor.object], range adaptor objects
+  template<class D>
+    requires is_class_v<D> && same_as<D, remove_cv_t<D>>
+  class range_adaptor_closure { };  // Since c++23
+
   // [view.interface], class template view_interface
   template<class D>
     requires is_class_v<D> && same_as<D, remove_cv_t<D>>
diff --git a/libcxx/test/std/ranges/range.adaptors/range.adaptor.object/range_adaptor_closure.pass.cpp b/libcxx/test/std/ranges/range.adaptors/range.adaptor.object/range_adaptor_closure.pass.cpp
new file mode 100644
index 00000000000000..546a8626c80e18
--- /dev/null
+++ b/libcxx/test/std/ranges/range.adaptors/range.adaptor.object/range_adaptor_closure.pass.cpp
@@ -0,0 +1,128 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+
+// std::ranges::range_adaptor_closure;
+
+#include <ranges>
+
+#include <algorithm>
+#include <vector>
+
+#include "test_range.h"
+
+using range_t = std::vector<int>;
+
+template <class T>
+concept RangeAdaptorClosure =
+    CanBePiped<range_t, T&> && CanBePiped<range_t, const T&> && CanBePiped<range_t, T&&> &&
+    CanBePiped<range_t, const T&&>;
+
+struct callable : std::ranges::range_adaptor_closure<callable> {
+  static void operator()(const range_t&) {}
+};
+static_assert(RangeAdaptorClosure<callable>);
+
+// `not_callable_1` doesn't have an `operator()`
+struct not_callable_1 : std::ranges::range_adaptor_closure<not_callable_1> {};
+static_assert(!RangeAdaptorClosure<not_callable_1>);
+
+// `not_callable_2` doesn't have an `operator()` that accepts a `range` argument
+struct not_callable_2 : std::ranges::range_adaptor_closure<not_callable_2> {
+  static void operator()() {}
+};
+static_assert(!RangeAdaptorClosure<not_callable_2>);
+
+// `not_derived_from_1` doesn't derive from `std::ranges::range_adaptor_closure`
+struct not_derived_from_1 {
+  static void operator()(const range_t&) {}
+};
+static_assert(!RangeAdaptorClosure<not_derived_from_1>);
+
+// `not_derived_from_2` doesn't publicly derive from `std::ranges::range_adaptor_closure`
+struct not_derived_from_2 : private std::ranges::range_adaptor_closure<not_derived_from_2> {
+  static void operator()(const range_t&) {}
+};
+static_assert(!RangeAdaptorClosure<not_derived_from_2>);
+
+// `not_derived_from_3` doesn't derive from the correct specialization of `std::ranges::range_adaptor_closure`
+struct not_derived_from_3 : std::ranges::range_adaptor_closure<callable> {
+  static void operator()(const range_t&) {}
+};
+static_assert(!RangeAdaptorClosure<not_derived_from_3>);
+
+// `not_derived_from_4` doesn't derive from a unique `std::ranges::range_adaptor_closure`
+struct not_derived_from_4
+    : std::ranges::range_adaptor_closure<not_derived_from_4>,
+      std::ranges::range_adaptor_closure<callable> {
+  static void operator()(const range_t&) {}
+};
+static_assert(!RangeAdaptorClosure<not_derived_from_4>);
+
+// `is_range` models `range`
+struct is_range : std::ranges::range_adaptor_closure<is_range> {
+  static void operator()(const range_t&) {}
+  int* begin() const { return nullptr; }
+  int* end() const { return nullptr; }
+};
+static_assert(std::ranges::range<is_range> && std::ranges::range<const is_range>);
+static_assert(!RangeAdaptorClosure<is_range>);
+
+// user-defined range adaptor closure object
+struct negate_fn : std::ranges::range_adaptor_closure<negate_fn> {
+  template <std::ranges::range Range>
+  static constexpr decltype(auto) operator()(Range&& range) {
+    return std::forward<Range>(range) | std::views::transform([](auto element) { return -element; });
+  }
+};
+static_assert(RangeAdaptorClosure<negate_fn>);
+constexpr auto negate = negate_fn{};
+
+// user-defined range adaptor closure object
+struct plus_1_fn : std::ranges::range_adaptor_closure<plus_1_fn> {
+  template <std::ranges::range Range>
+  static constexpr decltype(auto) operator()(Range&& range) {
+    return std::forward<Range>(range) | std::views::transform([](auto element) { return element + 1; });
+  }
+};
+static_assert(RangeAdaptorClosure<plus_1_fn>);
+constexpr auto plus_1 = plus_1_fn{};
+
+constexpr bool test() {
+  const std::vector<int> n{1, 2, 3, 4, 5};
+  const std::vector<int> n_negate{-1, -2, -3, -4, -5};
+  const std::vector<int> n_negate_plus_1{0, -1, -2, -3, -4};
+  const std::vector<int> n_plus_1_negate{-2, -3, -4, -5, -6};
+
+  assert(std::ranges::equal(n | negate, n_negate));
+  assert(std::ranges::equal(negate(n), n_negate));
+
+  assert(std::ranges::equal(n | negate | negate, n));
+  assert(std::ranges::equal(n | (negate | negate), n));
+  assert(std::ranges::equal((n | negate) | negate, n));
+  assert(std::ranges::equal(negate(n) | negate, n));
+  assert(std::ranges::equal(negate(n | negate), n));
+  assert(std::ranges::equal((negate | negate)(n), n));
+  assert(std::ranges::equal(negate(negate(n)), n));
+
+  assert(std::ranges::equal(n | negate | plus_1, n_negate_plus_1));
+  assert(std::ranges::equal(
+      n | plus_1 | std::views::transform([](auto element) { return element; }) | negate, n_plus_1_negate));
+
+  assert(std::ranges::equal(n | plus_1 | negate, n_plus_1_negate));
+  assert(std::ranges::equal(n | std::views::reverse | negate | plus_1 | std::views::reverse, n_negate_plus_1));
+  return true;
+}
+
+int main(int, char**) {
+  test();
+  static_assert(test());
+
+  return 0;
+}

@var-const var-const added the ranges Issues related to `<ranges>` label Apr 17, 2024
Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

Thanks for the patch! I have some comments but this looks pretty good.

I had a partial implementation of this locally but this patch is more complete and better overall, thanks for picking it up.

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! The CI job that's failing has been fixed on trunk so this is good to go.

@ldionne ldionne merged commit c108653 into llvm:main Apr 23, 2024
51 of 52 checks passed
@xiaoyang-sde xiaoyang-sde deleted the range_adaptors branch April 23, 2024 15:33
@stripe2933
Copy link
Contributor

The definition range_adaptor_closure have to be exported via this file (line 142).

@xiaoyang-sde
Copy link
Member Author

The definition range_adaptor_closure have to be exported via this file (line 142).

Thanks for pointing this out! I fixed it in #89793.

ldionne pushed a commit that referenced this pull request Apr 23, 2024
This patch exports the `std::ranges::range_adaptor_closure` class
template implemented in #89148 from the C++ Modules file.
JMazurkiewicz added a commit to JMazurkiewicz/llvm-project that referenced this pull request May 13, 2024
* Some docs should've been updated in llvm#74534, this PR does this.
* llvm#89148 should've bumped `__cpp_lib_ranges` macro to 202211 (value mentioned by "P2602R2 Poison Pills are Too Toxic")
* Drive-by: fix order of notes in `Cxx23.rst`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. ranges Issues related to `<ranges>`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants