[gcc r11-8547] libstdc++: Simplify range adaptors' forwarding of bound args [PR100577]

[gcc r11-8547] libstdc++: Simplify range adaptors' forwarding of bound args [PR100577]

[Patrick Palka]

https://gcc.gnu.org/g:80495610eb8c3b1395073cebd26a079d81b73403

commit r11-8547-g80495610eb8c3b1395073cebd26a079d81b73403
Author: Patrick Palka <ppalka@redhat.com>
Date:   Thu Jun 3 09:49:30 2021 -0400

    libstdc++: Simplify range adaptors' forwarding of bound args [PR100577]
    
    r11-8053 rewrote the range adaptor implementation in conformance with
    P2281R1, making partial application act like a SFINAE-friendly perfect
    forwarding call wrapper.  Making SFINAE-friendliness coexist with
    perfect forwarding here requires adding fallback deleted operator()
    overloads (as described in e.g. section 5.5 of P0847R6).  But
    unfortunately, as reported in PR100577, this necessary technique of
    using of deleted overloads regresses diagnostics for ill-formed calls to
    partially applied range adaptors in GCC.
    
    Although GCC's diagnostics can arguably be improved here by having the
    compiler explain why the other candidates weren't viable when overload
    resolution selects a deleted candidate, we can also largely work around
    this on the library side (and achieve more concise diagnostics than by
    a frontend-side improvement alone) if we take advantage of the
    observation that not all range adaptors need perfect forwarding call
    wrapper semantics, in fact only views::split currently needs it, because
    all other range adaptors either take no extra arguments or only
    arguments that are expected to be freely/cheaply copyable, e.g. function
    objects and integer-like types.  (The discussion section in P2281R1 goes
    into detail about why views::split is special.)
    
    To that end, this introduces opt-in flags for denoting a range adaptor
    as having "simple" extra arguments (in the case of a range adaptor
    non-closure) or having a "simple" call operator (in the case of a range
    adaptor closure).  These flags are then used to conditionally simplify
    the operator() for the generic _Partial and _Pipe class templates, down
    from needing three overloads thereof (including one defined as deleted)
    to just needing a single overload.  The end result is that diagnostic
    quality is restored for all adaptors except for views::split, and
    diagnostics for the adaptors are generally made more concise since
    there's only a single _Partial/_Pipe overload to diagnose instead of
    three of them.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/100577
            * include/std/ranges (_RangeAdaptorClosure): Document
            _S_has_simple_call_op mechanism.
            (_RangeAdaptor): Document _S_has_simple_extra_args mechanism.
            (__closure_has_simple_call_op): New concept.
            (__adaptor_has_simple_extra_args): Likewise.
            (_Partial<_Adaptor, _Args...>): New partial specialization.
            (_Partial<_Adaptor, _Arg>): Likewise.
            (_Pipe<_Lhs, _Rhs>): Likewise.
            (views::_All::_S_has_simple_call_op): Define to true.
            (views::_Filter::_S_has_simple_extra_args): Likewise.
            (views::_Transform::_S_has_simple_extra_args): Likewise.
            (views::_Take::_S_has_simple_extra_args): Likewise.
            (views::_TakeWhile::_S_has_simple_extra_args): Likewise.
            (views::_Drop::_S_has_simple_extra_args): Likewise.
            (views::_DropWhile::_S_has_simple_extra_args): Likewise.
            (views::_Join::_S_has_simple_call_op): Likewise.
            (views::_Split): Document why we don't define
            _S_has_simple_extra_args to true for this adaptor.
            (views::_Common::_S_has_simple_call_op): Define to true.
            (views::_Reverse::_S_has_simple_call_op): Likewise.
            (views::_Elements::_S_has_simple_call_op): Likewise.
            * testsuite/std/ranges/adaptors/100577.cc: New test.
    
    (cherry picked from commit 57ed620ebfa4275d04efeec973e88f506b0e3ba8)

Diff:
---
 libstdc++-v3/include/std/ranges                    | 119 ++++++++++++++++++++-
 .../testsuite/std/ranges/adaptors/100577.cc        | 101 +++++++++++++++++
 2 files changed, 219 insertions(+), 1 deletion(-)

diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 9ba28afbfce..b3173f4f7c7 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -760,6 +760,10 @@ namespace views::__adaptor
     struct _Pipe;
 
   // The base class of every range adaptor closure.
+  //
+  // The derived class should define the optional static data member
+  // _S_has_simple_call_op to true if the behavior of this adaptor is
+  // independent of the constness/value category of the adaptor object.
   struct _RangeAdaptorClosure
   {
     // range | adaptor is equivalent to adaptor(range).
@@ -785,6 +789,10 @@ namespace views::__adaptor
   // The static data member _Derived::_S_arity must contain the total number of
   // arguments that the adaptor takes, and the class _Derived must introduce
   // _RangeAdaptor::operator() into the class scope via a using-declaration.
+  //
+  // The optional static data member _Derived::_S_has_simple_extra_args should
+  // be defined to true if the behavior of this adaptor is independent of the
+  // constness/value category of the extra arguments.
   template<typename _Derived>
     struct _RangeAdaptor
     {
@@ -799,6 +807,17 @@ namespace views::__adaptor
 	}
     };
 
+  // True if the range adaptor closure _Adaptor has a simple operator(), i.e.
+  // one that's not overloaded according to constness or value category of the
+  // _Adaptor object.
+  template<typename _Adaptor>
+    concept __closure_has_simple_call_op = _Adaptor::_S_has_simple_call_op;
+
+  // True if the behavior of the range adaptor non-closure _Adaptor is
+  // independent of the value category of its extra arguments.
+  template<typename _Adaptor>
+    concept __adaptor_has_simple_extra_args = _Adaptor::_S_has_simple_extra_args;
+
   // A range adaptor closure that represents partial application of
   // the range adaptor _Adaptor with arguments _Args.
   template<typename _Adaptor, typename... _Args>
@@ -812,7 +831,7 @@ namespace views::__adaptor
       { }
 
       // Invoke _Adaptor with arguments __r, _M_args... according to the
-      // value category of the range adaptor closure object.
+      // value category of this _Partial object.
       template<typename _Range>
 	requires __adaptor_invocable<_Adaptor, _Range, const _Args&...>
 	constexpr auto
@@ -869,6 +888,59 @@ namespace views::__adaptor
 	operator()(_Range&& __r) const && = delete;
     };
 
+  // Partial specialization of the primary template for the case where the extra
+  // arguments of the adaptor can always be safely forwarded by const reference.
+  // This lets us get away with a single operator() overload, which makes
+  // overload resolution failure diagnostics more concise.
+  template<typename _Adaptor, typename... _Args>
+    requires __adaptor_has_simple_extra_args<_Adaptor>
+    struct _Partial<_Adaptor, _Args...> : _RangeAdaptorClosure
+    {
+      tuple<_Args...> _M_args;
+
+      constexpr
+      _Partial(_Args... __args)
+	: _M_args(std::move(__args)...)
+      { }
+
+      // Invoke _Adaptor with arguments __r, const _M_args&... regardless
+      // of the value category of this _Partial object.
+      template<typename _Range>
+	requires __adaptor_invocable<_Adaptor, _Range, const _Args&...>
+	constexpr auto
+	operator()(_Range&& __r) const
+	{
+	  auto __forwarder = [&__r] (const auto&... __args) {
+	    return _Adaptor{}(std::forward<_Range>(__r), __args...);
+	  };
+	  return std::apply(__forwarder, _M_args);
+	}
+
+      static constexpr bool _S_has_simple_call_op = true;
+    };
+
+  // A lightweight specialization of the above template for the common case
+  // where _Adaptor accepts a single extra argument.
+  template<typename _Adaptor, typename _Arg>
+    requires __adaptor_has_simple_extra_args<_Adaptor>
+    struct _Partial<_Adaptor, _Arg> : _RangeAdaptorClosure
+    {
+      _Arg _M_arg;
+
+      constexpr
+      _Partial(_Arg __arg)
+	: _M_arg(std::move(__arg))
+      { }
+
+      template<typename _Range>
+	requires __adaptor_invocable<_Adaptor, _Range, const _Arg&>
+	constexpr auto
+	operator()(_Range&& __r) const
+	{ return _Adaptor{}(std::forward<_Range>(__r), _M_arg); }
+
+      static constexpr bool _S_has_simple_call_op = true;
+    };
+
   template<typename _Lhs, typename _Rhs, typename _Range>
     concept __pipe_invocable
       = requires { std::declval<_Rhs>()(std::declval<_Lhs>()(std::declval<_Range>())); };
@@ -904,6 +976,32 @@ namespace views::__adaptor
 	constexpr auto
 	operator()(_Range&& __r) const && = delete;
     };
+
+  // A partial specialization of the above primary template for the case where
+  // both adaptor operands have a simple operator().  This in turn lets us
+  // implement composition using a single simple operator(), which makes
+  // overload resolution failure diagnostics more concise.
+  template<typename _Lhs, typename _Rhs>
+    requires __closure_has_simple_call_op<_Lhs>
+      && __closure_has_simple_call_op<_Rhs>
+    struct _Pipe<_Lhs, _Rhs> : _RangeAdaptorClosure
+    {
+      [[no_unique_address]] _Lhs _M_lhs;
+      [[no_unique_address]] _Rhs _M_rhs;
+
+      constexpr
+      _Pipe(_Lhs __lhs, _Rhs __rhs)
+	: _M_lhs(std::move(__lhs)), _M_rhs(std::move(__rhs))
+      { }
+
+      template<typename _Range>
+	requires __pipe_invocable<const _Lhs&, const _Rhs&, _Range>
+	constexpr auto
+	operator()(_Range&& __r) const
+	{ return _M_rhs(_M_lhs(std::forward<_Range>(__r))); }
+
+      static constexpr bool _S_has_simple_call_op = true;
+    };
 } // namespace views::__adaptor
 
   template<range _Range> requires is_object_v<_Range>
@@ -985,6 +1083,8 @@ namespace views::__adaptor
 	  else
 	    return subrange{std::forward<_Range>(__r)};
 	}
+
+      static constexpr bool _S_has_simple_call_op = true;
     };
 
     inline constexpr _All all;
@@ -1438,6 +1538,7 @@ namespace views::__adaptor
 
       using _RangeAdaptor<_Filter>::operator();
       static constexpr int _S_arity = 2;
+      static constexpr bool _S_has_simple_extra_args = true;
     };
 
     inline constexpr _Filter filter;
@@ -1812,6 +1913,7 @@ namespace views::__adaptor
 
       using _RangeAdaptor<_Transform>::operator();
       static constexpr int _S_arity = 2;
+      static constexpr bool _S_has_simple_extra_args = true;
     };
 
     inline constexpr _Transform transform;
@@ -1992,6 +2094,7 @@ namespace views::__adaptor
 
       using _RangeAdaptor<_Take>::operator();
       static constexpr int _S_arity = 2;
+      static constexpr bool _S_has_simple_extra_args = true;
     };
 
     inline constexpr _Take take;
@@ -2111,6 +2214,7 @@ namespace views::__adaptor
 
       using _RangeAdaptor<_TakeWhile>::operator();
       static constexpr int _S_arity = 2;
+      static constexpr bool _S_has_simple_extra_args = true;
     };
 
     inline constexpr _TakeWhile take_while;
@@ -2230,6 +2334,7 @@ namespace views::__adaptor
 
       using _RangeAdaptor<_Drop>::operator();
       static constexpr int _S_arity = 2;
+      static constexpr bool _S_has_simple_extra_args = true;
     };
 
     inline constexpr _Drop drop;
@@ -2313,6 +2418,7 @@ namespace views::__adaptor
 
       using _RangeAdaptor<_DropWhile>::operator();
       static constexpr int _S_arity = 2;
+      static constexpr bool _S_has_simple_extra_args = true;
     };
 
     inline constexpr _DropWhile drop_while;
@@ -2670,6 +2776,8 @@ namespace views::__adaptor
 	  // 3474. Nesting join_views is broken because of CTAD
 	  return join_view<all_t<_Range>>{std::forward<_Range>(__r)};
 	}
+
+      static constexpr bool _S_has_simple_call_op = true;
     };
 
     inline constexpr _Join join;
@@ -3103,6 +3211,9 @@ namespace views::__adaptor
 
       using _RangeAdaptor<_Split>::operator();
       static constexpr int _S_arity = 2;
+      // The second argument of views::split is _not_ simple -- it can be a
+      // non-view range, the value category of which affects whether the call is
+      // well-formed.  So we must not define _S_has_simple_extra_args to true.
     };
 
     inline constexpr _Split split;
@@ -3242,6 +3353,8 @@ namespace views::__adaptor
 	  else
 	    return common_view{std::forward<_Range>(__r)};
 	}
+
+      static constexpr bool _S_has_simple_call_op = true;
     };
 
     inline constexpr _Common common;
@@ -3371,6 +3484,8 @@ namespace views::__adaptor
 	  else
 	    return reverse_view{std::forward<_Range>(__r)};
 	}
+
+      static constexpr bool _S_has_simple_call_op = true;
     };
 
     inline constexpr _Reverse reverse;
@@ -3754,6 +3869,8 @@ namespace views::__adaptor
 	  {
 	    return elements_view<all_t<_Range>, _Nm>{std::forward<_Range>(__r)};
 	  }
+
+	static constexpr bool _S_has_simple_call_op = true;
       };
 
     template<size_t _Nm>
diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/100577.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/100577.cc
new file mode 100644
index 00000000000..29176c8b7b0
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/ranges/adaptors/100577.cc
@@ -0,0 +1,101 @@
+// Copyright (C) 2021 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-options "-std=gnu++2a" }
+// { dg-do compile { target c++2a } }
+
+// PR libstdc++/100577
+
+#include <ranges>
+
+namespace ranges = std::ranges;
+namespace views = std::ranges::views;
+
+void
+test01()
+{
+  // Verify all multi-argument adaptors except for views::split are denoted
+  // to have simple extra arguments.
+  using views::__adaptor::__adaptor_has_simple_extra_args;
+  static_assert(__adaptor_has_simple_extra_args<decltype(views::transform)>);
+  static_assert(__adaptor_has_simple_extra_args<decltype(views::filter)>);
+  static_assert(__adaptor_has_simple_extra_args<decltype(views::drop)>);
+  static_assert(__adaptor_has_simple_extra_args<decltype(views::take)>);
+  static_assert(__adaptor_has_simple_extra_args<decltype(views::take_while)>);
+  static_assert(__adaptor_has_simple_extra_args<decltype(views::drop_while)>);
+  static_assert(!__adaptor_has_simple_extra_args<decltype(views::split)>);
+
+  // Verify all adaptor closures except for views::split(pattern) have a simple
+  // operator().
+  using views::__adaptor::__closure_has_simple_call_op;
+  __closure_has_simple_call_op auto a00 = views::all;
+  __closure_has_simple_call_op auto a01 = views::transform(std::identity{});
+  __closure_has_simple_call_op auto a02 = views::filter(std::identity{});
+  __closure_has_simple_call_op auto a03 = views::drop(42);
+  __closure_has_simple_call_op auto a04 = views::take(42);
+  __closure_has_simple_call_op auto a05 = views::take_while(std::identity{});
+  __closure_has_simple_call_op auto a06 = views::drop_while(std::identity{});
+  __closure_has_simple_call_op auto a07 = views::join;
+  __closure_has_simple_call_op auto a08 = views::common;
+  __closure_has_simple_call_op auto a09 = views::reverse;
+  __closure_has_simple_call_op auto a10 = views::keys;
+  // Verify composition of simple closures is simple.
+  __closure_has_simple_call_op auto b
+    = (a00 | a01) | (a02 | a03) | (a04 | a05 | a06) | (a07 | a08 | a09 | a10);
+
+  // Verify views::split is the exception.
+  auto a11 = views::split(' ');
+  static_assert(!__closure_has_simple_call_op<decltype(a11)>);
+  static_assert(!__closure_has_simple_call_op<decltype(a11 | a00)>);
+  static_assert(!__closure_has_simple_call_op<decltype(a00 | a11)>);
+}
+
+void
+test02()
+{
+  // Range adaptor closures with a simple operator() aren't implemented using a
+  // fallback deleted overload, so when a call is ill-formed overload resolution
+  // fails.
+  extern int x[10];
+  auto badarg = nullptr;
+  views::transform(badarg)(x); // { dg-error "no match" }
+  views::filter(badarg)(x); // { dg-error "no match" }
+  views::take(badarg)(x); // { dg-error "no match" }
+  views::drop(badarg)(x); // { dg-error "no match" }
+  views::take_while(badarg)(x); // { dg-error "no match" }
+  views::drop_while(badarg)(x); // { dg-error "no match" }
+
+  (views::transform(badarg) | views::all)(x); // { dg-error "no match" }
+  (views::filter(badarg) | views::all)(x); // { dg-error "no match" }
+  (views::take(badarg) | views::all)(x); // { dg-error "no match" }
+  (views::drop(badarg) | views::all)(x); // { dg-error "no match" }
+  (views::take_while(badarg) | views::all)(x); // { dg-error "no match" }
+  (views::drop_while(badarg) | views::all)(x); // { dg-error "no match" }
+
+  // In practice, range adaptor closures with non-simple operator() are
+  // implemented using a fallback deleted overload, so when a call is
+  // ill-formed overload resolution succeeds but selects the deleted overload
+  // (but only when the closure is invoked as an rvalue).
+  views::split(badarg)(x); // { dg-error "deleted function" }
+  (views::split(badarg) | views::all)(x); // { dg-error "deleted function" }
+  auto a0 = views::split(badarg);
+  a0(x); // { dg-error "no match" };
+  auto a1 = a0 | views::all;
+  a1(x); // { dg-error "no match" }
+}
+
+// { dg-prune-output "in requirements" }