libstdc++/ranges: make _RangeAdaptorClosure befriend operator|

Message ID 20241127154246.2535580-1-ppalka@redhat.com
State New
Headers
Series libstdc++/ranges: make _RangeAdaptorClosure befriend operator| |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_build--master-arm fail Patch failed to apply

Commit Message

Patrick Palka Nov. 27, 2024, 3:42 p.m. UTC
  Tested on x86_64-pc-linux-gnu, does this look OK for trunk?

-- >8 --

This declares the range adaptor pipe operators a friend of the
_RangeAdaptorClosure base class so that the std module doesn't need to
export them for ADL to find them.

Note that we deliberately don't define these pipe operators as hidden
friends, see r14-3293-g4a6f3676e7dd9e.

libstdc++-v3/ChangeLog:

	* include/std/ranges (views::__adaptor::_RangeAdaptorClosure):
	Befriend both operator| overloads.
---
 libstdc++-v3/include/std/ranges | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)
  

Comments

Jonathan Wakely Nov. 27, 2024, 3:51 p.m. UTC | #1
On Wed, 27 Nov 2024 at 15:43, Patrick Palka <ppalka@redhat.com> wrote:
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?

OK

>
> -- >8 --
>
> This declares the range adaptor pipe operators a friend of the
> _RangeAdaptorClosure base class so that the std module doesn't need to
> export them for ADL to find them.
>
> Note that we deliberately don't define these pipe operators as hidden
> friends, see r14-3293-g4a6f3676e7dd9e.
>
> libstdc++-v3/ChangeLog:
>
>         * include/std/ranges (views::__adaptor::_RangeAdaptorClosure):
>         Befriend both operator| overloads.
> ---
>  libstdc++-v3/include/std/ranges | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
> index 5153dcc26c4..9d30e3a8e9d 100644
> --- a/libstdc++-v3/include/std/ranges
> +++ b/libstdc++-v3/include/std/ranges
> @@ -949,8 +949,7 @@ namespace views::__adaptor
>    // _S_has_simple_call_op to true if the behavior of this adaptor is
>    // independent of the constness/value category of the adaptor object.
>    template<typename _Derived>
> -    struct _RangeAdaptorClosure
> -    { };
> +    struct _RangeAdaptorClosure;
>
>    template<typename _Tp, typename _Up>
>      requires (!same_as<_Tp, _RangeAdaptorClosure<_Up>>)
> @@ -984,6 +983,26 @@ namespace views::__adaptor
>      }
>  #pragma GCC diagnostic pop
>
> +  template<typename _Derived>
> +    struct _RangeAdaptorClosure
> +    {
> +      // In non-modules compilation ADL finds these operator| either way and
> +      // the friend declarations are redundant.  But with the std module these
> +      // friend declarations enable ADL to find these operators without having
> +      // to export them.
> +      template<typename _Self, typename _Range>
> +       requires __is_range_adaptor_closure<_Self>
> +         && __adaptor_invocable<_Self, _Range>
> +       friend constexpr auto
> +       operator|(_Range&& __r, _Self&& __self);
> +
> +      template<typename _Lhs, typename _Rhs>
> +       requires __is_range_adaptor_closure<_Lhs>
> +         && __is_range_adaptor_closure<_Rhs>
> +       friend constexpr auto
> +       operator|(_Lhs&& __lhs, _Rhs&& __rhs);
> +    };
> +
>    // The base class of every range adaptor non-closure.
>    //
>    // The static data member _Derived::_S_arity must contain the total number of
> --
> 2.47.1.313.gcc01bad4a9
>
  
Patrick Palka Nov. 27, 2024, 5:02 p.m. UTC | #2
On Wed, 27 Nov 2024, Jonathan Wakely wrote:

> On Wed, 27 Nov 2024 at 15:43, Patrick Palka <ppalka@redhat.com> wrote:
> >
> > Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
> 
> OK

Thanks, I noticed Jason already pushed his module std fixes patch so I
went ahead and removed the now unnecessary export as well.  This is
what I ultimately pushed:

-- >8 --

Subject: [PATCH] libstdc++/ranges: make _RangeAdaptorClosure befriend
 operator|

This declares the range adaptor pipe operators a friend of the
_RangeAdaptorClosure base class so that the std module doesn't need to
export them for ADL to find them.

Note that we deliberately don't define these pipe operators as hidden
friends, see r14-3293-g4a6f3676e7dd9e.

libstdc++-v3/ChangeLog:

	* include/std/ranges (views::__adaptor::_RangeAdaptorClosure):
	Befriend both operator| overloads.
	* src/c++23/std.cc.in: Don't export views::__adaptor::operator|.

Reviewed-by: Jonathan Wakely <jwakely@redhat.com>
---
 libstdc++-v3/include/std/ranges  | 23 +++++++++++++++++++++--
 libstdc++-v3/src/c++23/std.cc.in |  6 ------
 2 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 5153dcc26c4..f4b89778479 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -949,8 +949,7 @@ namespace views::__adaptor
   // _S_has_simple_call_op to true if the behavior of this adaptor is
   // independent of the constness/value category of the adaptor object.
   template<typename _Derived>
-    struct _RangeAdaptorClosure
-    { };
+    struct _RangeAdaptorClosure;
 
   template<typename _Tp, typename _Up>
     requires (!same_as<_Tp, _RangeAdaptorClosure<_Up>>)
@@ -984,6 +983,26 @@ namespace views::__adaptor
     }
 #pragma GCC diagnostic pop
 
+  template<typename _Derived>
+    struct _RangeAdaptorClosure
+    {
+      // In non-modules compilation ADL finds these operators either way and
+      // the friend declarations are redundant.  But with the std module these
+      // friend declarations enable ADL to find these operators without having
+      // to export them.
+      template<typename _Self, typename _Range>
+	requires __is_range_adaptor_closure<_Self>
+	  && __adaptor_invocable<_Self, _Range>
+	friend constexpr auto
+	operator|(_Range&& __r, _Self&& __self);
+
+      template<typename _Lhs, typename _Rhs>
+	requires __is_range_adaptor_closure<_Lhs>
+	  && __is_range_adaptor_closure<_Rhs>
+	friend constexpr auto
+	operator|(_Lhs&& __lhs, _Rhs&& __rhs);
+    };
+
   // The base class of every range adaptor non-closure.
   //
   // The static data member _Derived::_S_arity must contain the total number of
diff --git a/libstdc++-v3/src/c++23/std.cc.in b/libstdc++-v3/src/c++23/std.cc.in
index 7d787a55555..16e66c3d921 100644
--- a/libstdc++-v3/src/c++23/std.cc.in
+++ b/libstdc++-v3/src/c++23/std.cc.in
@@ -2366,12 +2366,6 @@ export namespace std
     using ranges::concat_view;
     namespace views { using views::concat; }
 #endif
-
-    // FIXME can we avoid this export using friends?
-    namespace views::__adaptor
-    {
-      using __adaptor::operator|;
-    }
   }
 }
  

Patch

diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 5153dcc26c4..9d30e3a8e9d 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -949,8 +949,7 @@  namespace views::__adaptor
   // _S_has_simple_call_op to true if the behavior of this adaptor is
   // independent of the constness/value category of the adaptor object.
   template<typename _Derived>
-    struct _RangeAdaptorClosure
-    { };
+    struct _RangeAdaptorClosure;
 
   template<typename _Tp, typename _Up>
     requires (!same_as<_Tp, _RangeAdaptorClosure<_Up>>)
@@ -984,6 +983,26 @@  namespace views::__adaptor
     }
 #pragma GCC diagnostic pop
 
+  template<typename _Derived>
+    struct _RangeAdaptorClosure
+    {
+      // In non-modules compilation ADL finds these operator| either way and
+      // the friend declarations are redundant.  But with the std module these
+      // friend declarations enable ADL to find these operators without having
+      // to export them.
+      template<typename _Self, typename _Range>
+	requires __is_range_adaptor_closure<_Self>
+	  && __adaptor_invocable<_Self, _Range>
+	friend constexpr auto
+	operator|(_Range&& __r, _Self&& __self);
+
+      template<typename _Lhs, typename _Rhs>
+	requires __is_range_adaptor_closure<_Lhs>
+	  && __is_range_adaptor_closure<_Rhs>
+	friend constexpr auto
+	operator|(_Lhs&& __lhs, _Rhs&& __rhs);
+    };
+
   // The base class of every range adaptor non-closure.
   //
   // The static data member _Derived::_S_arity must contain the total number of