| Message ID | 20210514182749.602087-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | libstdc++: Simplify range adaptors' forwarding of bound args when possible | expand |
On Fri, 14 May 2021, Patrick Palka wrote: > r11-8053 rewrote the range adaptor implementation in conformance with > P2281, 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 wg21.link/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 of P2281 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. Ping. > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk/11? > > 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. > --- > libstdc++-v3/include/std/ranges | 119 +++++++++++++++++- > .../testsuite/std/ranges/adaptors/100577.cc | 101 +++++++++++++++ > 2 files changed, 219 insertions(+), 1 deletion(-) > create mode 100644 libstdc++-v3/testsuite/std/ranges/adaptors/100577.cc > > diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges > index 48100e9d7f2..0f69d4f0839 100644 > --- a/libstdc++-v3/include/std/ranges > +++ b/libstdc++-v3/include/std/ranges > @@ -756,6 +756,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). > @@ -781,6 +785,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 > { > @@ -795,6 +803,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> > @@ -808,7 +827,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 > @@ -865,6 +884,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>())); }; > @@ -900,6 +972,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> > @@ -981,6 +1079,8 @@ namespace views::__adaptor > else > return subrange{std::forward<_Range>(__r)}; > } > + > + static constexpr bool _S_has_simple_call_op = true; > }; > > inline constexpr _All all; > @@ -1353,6 +1453,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; > @@ -1727,6 +1828,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; > @@ -1907,6 +2009,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; > @@ -2026,6 +2129,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; > @@ -2145,6 +2249,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; > @@ -2228,6 +2333,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; > @@ -2645,6 +2751,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; > @@ -3078,6 +3186,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; > @@ -3217,6 +3328,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; > @@ -3346,6 +3459,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; > @@ -3723,6 +3838,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..719ba15d947 > --- /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(foo) 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" } > -- > 2.31.1.606.gdf6c4f722c > >
On 14/05/21 14:27 -0400, Patrick Palka via Libstdc++ wrote: >r11-8053 rewrote the range adaptor implementation in conformance with >P2281, 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 wg21.link/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 of P2281 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. > >Tested on x86_64-pc-linux-gnu, does this look OK for trunk/11? OK for trunk and 11. Is there any benefit in using [[no_unique_address]] for _Partial::_M_args too?
diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges index 48100e9d7f2..0f69d4f0839 100644 --- a/libstdc++-v3/include/std/ranges +++ b/libstdc++-v3/include/std/ranges @@ -756,6 +756,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). @@ -781,6 +785,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 { @@ -795,6 +803,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> @@ -808,7 +827,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 @@ -865,6 +884,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>())); }; @@ -900,6 +972,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> @@ -981,6 +1079,8 @@ namespace views::__adaptor else return subrange{std::forward<_Range>(__r)}; } + + static constexpr bool _S_has_simple_call_op = true; }; inline constexpr _All all; @@ -1353,6 +1453,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; @@ -1727,6 +1828,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; @@ -1907,6 +2009,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; @@ -2026,6 +2129,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; @@ -2145,6 +2249,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; @@ -2228,6 +2333,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; @@ -2645,6 +2751,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; @@ -3078,6 +3186,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; @@ -3217,6 +3328,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; @@ -3346,6 +3459,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; @@ -3723,6 +3838,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..719ba15d947 --- /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(foo) 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" }