[Bug c++/99433] New: custom friend pipe-operator| conflicts with range adaptor?

[Bug c++/99433] New: custom friend pipe-operator| conflicts with range adaptor?

[gcc-bugs at marehr dot dialup.fu-berlin.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99433

            Bug ID: 99433
           Summary: custom friend pipe-operator| conflicts with range
                    adaptor?
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gcc-bugs at marehr dot dialup.fu-berlin.de
  Target Milestone: ---

Hello gcc-team,

the following code does not compile with gcc-11 any more, but did with gcc-10.

```c++
#include <ranges>
#include <vector>
template <typename underlying_adaptor_t>
struct deep {
    underlying_adaptor_t adaptor;

    template <typename range_t>
    friend auto operator|(range_t &range, deep const &me)
    {
        return me.adaptor(range[0]);
    }
};
inline auto const complement = deep{std::views::transform([](auto const nucl) {
return nucl + ' '; })};
std::vector<std::vector<char>> foo{};
auto v = foo | complement;
```

https://godbolt.org/z/9oEj9T

Thank you!

[Bug libstdc++/99433] [11 Regression] custom friend pipe-operator| conflicts with range adaptor?

[gcc-bugs at marehr dot dialup.fu-berlin.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99433

gcc-bugs at marehr dot dialup.fu-berlin.de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|custom friend               |[11 Regression] custom
                   |pipe-operator| conflicts    |friend pipe-operator|
                   |with range adaptor?         |conflicts with range
                   |                            |adaptor?
          Component|c++                         |libstdc++

--- Comment #1 from gcc-bugs at marehr dot dialup.fu-berlin.de ---
Hello gcc-team,

I got the code more recuced to:

```c++
namespace std
{
template <typename _Tp, typename _Up = _Tp &&> _Up __declval(int);
template <typename _Tp> auto declval() noexcept -> decltype(__declval<_Tp>(0));
} // namespace std

namespace std::ranges
{
struct view_base {};
template <typename _Derived>
class view_interface : public view_base {};
} // namespace std::ranges

namespace std::ranges::views::__adaptor
{

template <typename _Callable>
struct _RangeAdaptorClosure;

template <typename _Callable>
struct _RangeAdaptor
{
    _Callable _M_callable;

    constexpr _RangeAdaptor(const _Callable & __callable)
        : _M_callable{__callable}
    {}

    template <typename... _Args>
    constexpr auto operator()(_Args &&...__args) const
    {
        auto __closure = [... __args(__args)]<typename _Range>(_Range &&__r) {
            return _Callable{}(__r, __args...);
        };
        using _ClosureType = decltype(__closure);
        return _RangeAdaptorClosure{__closure};
    }
};

template <typename _Callable>
struct _RangeAdaptorClosure : public _RangeAdaptor<_Callable>
{
    template <typename _Range>
        requires requires {declval<_Callable>()(declval<_Range>());}
    friend constexpr auto operator|(_Range &&__r, const _RangeAdaptorClosure
&__o);
};

template <typename _Callable>
_RangeAdaptorClosure(_Callable) -> _RangeAdaptorClosure<_Callable>;
} // namespace std::ranges::views::__adaptor

namespace std::ranges
{
template <typename _Vp, typename _Fp>
class transform_view : public view_interface<transform_view<_Vp, _Fp>> {};
} // namespace std::ranges

namespace std::ranges::views
{

inline constexpr __adaptor::_RangeAdaptor transform =
    [](auto &&__r, auto &&__f)
    {
        return transform_view{__r, __f};
    };
} // namespace std::ranges::views


namespace std
{
    namespace views = ranges::views;
    template <typename _Tp> class vector {};
} // namespace std

template <typename underlying_adaptor_t>
struct deep
{
    underlying_adaptor_t adaptor;

    template <typename range_t>
    friend auto operator|(range_t &range, deep const &me) {
        return 0;
    }
};

template <typename underlying_adaptor_t>
deep(underlying_adaptor_t && adaptor) -> deep<underlying_adaptor_t>;

inline auto const complement = deep{std::views::transform([]() {})};
std::vector<std::vector<char>> foo;
auto v = foo | complement;
```

See https://godbolt.org/z/51d9da

AFAIS the problem is that

```c++
template <typename _Range>
    requires requires {declval<_Callable>()(declval<_Range>());}
std::ranges::views::__adaptor::_RangeAdaptorClosure::operator|(_Range &&__r,
const _RangeAdaptorClosure &__o)
```

does not constraint the second argument to be `_RangeAdaptorClosure &__o`.

If I see this correctly, this is the same issue as in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97704 (which was marked invalid).

Just that this time your std-lib implementation is at fault. I still find it
insane that a non-template argument that does not fit is somehow considered in
a requires expression, since this effectively forbids to use constraints on non
template or partial template functions, but it is as it is, and it would be
cool if the std-lib implementation plays by the same rules.

(Why isn't a constraint with the type added implicitly to the requires clause
in these cases? That seems to be a workaround anyway...)

[Bug libstdc++/99433] [11 Regression] custom friend pipe-operator| conflicts with range adaptor?

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99433

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.0

[Bug libstdc++/99433] [11 Regression] custom friend pipe-operator| conflicts with range adaptor?

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99433

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ppalka at gcc dot gnu.org
     Ever confirmed|0                           |1
           Assignee|unassigned at gcc dot gnu.org      |ppalka at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2021-03-17

--- Comment #2 from Patrick Palka <ppalka at gcc dot gnu.org> ---
I think the underlying problem here is excessive instantiation due to usage of
deduced return types, which leads to hard errors during overload resolution.

If I give the lambda in your testcase a concrete return type decltype(nucl + '
'), and apply the following change to <ranges> (giving concrete return types to
a couple of lambdas therein) then we accept the testcase.

diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges  
index 1be74beb860..9fd647c144f 100644                                           
--- a/libstdc++-v3/include/std/ranges                                           
+++ b/libstdc++-v3/include/std/ranges                                           
@@ -803,7 +803,11 @@ namespace views                                            
                // otherwise capture by value.                                  
                auto __closure                                                  
                  = [...__args(__maybe_refwrap(std::forward<_Args>(__args)))]   
-                   <typename _Range> (_Range&& __r) {                          
+                   <typename _Range> (_Range&& __r)                            
+                 -> decltype(_Callable{}(std::forward<_Range>(__r),            
+                              (static_cast<unwrap_reference_t                  
+                                           <remove_const_t<decltype(__args)>>> 
+                               (__args))...)) {                                
                      // This static_cast has two purposes: it forwards a       
                      // reference_wrapper<T> capture as a T&, and otherwise    
                      // forwards the captured argument as an rvalue.           
@@ -1655,6 +1659,7 @@ namespace views                                           
   {                                                                            
     inline constexpr __adaptor::_RangeAdaptor transform                        
       = [] <viewable_range _Range, typename _Fp> (_Range&& __r, _Fp&& __f)     
+      -> decltype(transform_view{std::forward<_Range>(__r),
std::forward<_Fp>(__f)})                                                        
       {                                                                        
        return transform_view{std::forward<_Range>(__r),
std::forward<_Fp>(__f)};                                                        
       };                                                                       


For maximum SFINAE friendliness, all the range adaptors objects should probably
be changed to avoid using deduced return types, in a manner similar to the
above.  While we're at it, we should make them conditionally noexcept.

[Bug libstdc++/99433] [11 Regression] custom friend pipe-operator| conflicts with range adaptor?

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99433

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1
           Keywords|                            |rejects-valid

[Bug libstdc++/99433] [11 Regression] custom friend pipe-operator| conflicts with range adaptor?

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99433

--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:a25321ca06f61e5aeadc00923249f83af72059c5

commit r11-8053-ga25321ca06f61e5aeadc00923249f83af72059c5
Author: Patrick Palka <ppalka@redhat.com>
Date:   Thu Apr 8 10:40:19 2021 -0400

    libstdc++: Reimplement range adaptors [PR99433]

    This rewrites our range adaptor implementation for more comprehensible
    error messages, improved SFINAE behavior and conformance to P2281.

    The diagnostic improvements mostly come from using appropriately named
    functors instead of lambdas in the generic implementation of partial
    application and composition of range adaptors, and in the definition of
    each of the standard range adaptors.  This makes their pretty printed
    types much shorter and more self-descriptive.

    The improved SFINAE behavior comes from constraining the range adaptors'
    member functions appropriately.  This improvement fixes PR99433, and is
    also necessary in order to implement the wording changes of P2281.

    Finally, P2281 clarified that partial application and composition of
    range adaptors behaves like a perfect forwarding call wrapper.  This
    patch implements this, except that we don't bother adding overloads for
    forwarding captured state entities as non-const lvalues, since it seems
    sufficient to handle the const lvalue and non-const rvalue cases for now,
    given the current set of standard range adaptors.  But such overloads
    can be easily added if they turn out to be needed.

    libstdc++-v3/ChangeLog:

            PR libstdc++/99433
            * include/std/ranges (__adaptor::__maybe_refwrap): Remove.
            (__adaptor::__adaptor_invocable): New concept.
            (__adaptor::__adaptor_partial_app_viable): New concept.
            (__adaptor::_RangeAdaptorClosure): Rewrite, turning it into a
            non-template base class.
            (__adaptor::_RangeAdaptor): Rewrite, turning it into a CRTP base
            class template.
            (__adaptor::_Partial): New class template that represents
            partial application of a range adaptor non-closure.
            (__adaptor::__pipe_invocable): New concept.
            (__adaptor::_Pipe): New class template.
            (__detail::__can_ref_view): New concept.
            (__detail::__can_subrange): New concept.
            (all): Replace the lambda here with ...
            (_All): ... this functor.  Add appropriate constraints.
            (__detail::__can_filter_view): New concept.
            (filter, _Filter): As in all/_All.
            (__detail::__can_transform): New concept.
            (transform, _Transform): As in all/_All.
            (__detail::__can_take_view): New concept.
            (take, _Take): As in all/_All.
            (__detail::__can_take_while_view): New concept.
            (take_while, _TakeWhile): As in all/_All.
            (__detail::__can_drop_view): New concept.
            (drop, _Drop): As in all/_All.
            (__detail::__can_drop_while_view): New concept.
            (drop_while, _DropWhile): As in all/_All.
            (__detail::__can_join_view): New concept.
            (join, _Join): As in all/_All.
            (__detail::__can_split_view): New concept.
            (split, _Split): As in all/_All.  Rename template parameter
            _Fp to _Pattern.
            (__detail::__already_common): New concept.
            (__detail::__can_common_view): New concept.
            (common, _Common): As in all/_All.
            (__detail::__can_reverse_view): New concept.
            (reverse, _Reverse): As in all/_All.
            (__detail::__can_elements_view): New concept.
            (elements, _Elements): As in all/_All.
            (keys, values): Adjust.
            * testsuite/std/ranges/adaptors/99433.cc: New test.
            * testsuite/std/ranges/adaptors/all.cc: No longer expect that
            adding empty range adaptor closure objects to a pipeline doesn't
            increase the size of the pipeline.
            (test05): New test.
            * testsuite/std/ranges/adaptors/common.cc (test03): New test.
            * testsuite/std/ranges/adaptors/drop.cc (test09): New test.
            * testsuite/std/ranges/adaptors/drop_while.cc (test04): New test.
            * testsuite/std/ranges/adaptors/elements.cc (test04): New test.
            * testsuite/std/ranges/adaptors/filter.cc (test06): New test.
            * testsuite/std/ranges/adaptors/join.cc (test09): New test.
            * testsuite/std/ranges/adaptors/p2281.cc: New test.
            * testsuite/std/ranges/adaptors/reverse.cc (test07): New test.
            * testsuite/std/ranges/adaptors/split.cc (test01, test04):
            Adjust.
            (test09): New test.
            * testsuite/std/ranges/adaptors/split_neg.cc (test01): Adjust
            expected error message.
            (test02): Likewise.  Extend test.
            * testsuite/std/ranges/adaptors/take.cc (test06): New test.
            * testsuite/std/ranges/adaptors/take_while.cc (test05): New test.
            * testsuite/std/ranges/adaptors/transform.cc (test07, test08):
            New test.

[Bug libstdc++/99433] [11 Regression] custom friend pipe-operator| conflicts with range adaptor?

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99433

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Fixed for GCC 11 as part of a larger rewriting of the range adaptors, thanks
for the report.

[Bug libstdc++/99433] [11 Regression] custom friend pipe-operator| conflicts with range adaptor?

[gcc-bugs at marehr dot dialup.fu-berlin.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99433

--- Comment #5 from gcc-bugs at marehr dot dialup.fu-berlin.de ---
Thank you for the fix, but the following code does not compile any more:

```c++
#include <list>
#include <ranges>

int main()
{
  std::list<char> list;

  constexpr auto drop = []<std::ranges::viewable_range urng_t>(urng_t &&
urange, size_t drop_size)
  {
    // does not work:
    return std::forward<urng_t>(urange) | std::views::drop(drop_size);

    // does work:
    // return std::forward<urng_t>(urange) | std::views::drop(0);
  };
  drop(list, 0);
}
```

Should I open a new issue?

[Bug libstdc++/99433] [11 Regression] custom friend pipe-operator| conflicts with range adaptor?

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99433

--- Comment #6 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to gcc-bugs from comment #5)
> Thank you for the fix, but the following code does not compile any more:
> 
> ```c++
> #include <list>
> #include <ranges>
> 
> int main()
> {
>   std::list<char> list;
> 
>   constexpr auto drop = []<std::ranges::viewable_range urng_t>(urng_t &&
> urange, size_t drop_size)
>   {
>     // does not work:
>     return std::forward<urng_t>(urange) | std::views::drop(drop_size);
> 
>     // does work:
>     // return std::forward<urng_t>(urange) | std::views::drop(0);
>   };
>   drop(list, 0);
> }
> ```
> 
> Should I open a new issue?

Reduced:

#include <list>
#include <ranges>

int main() {
  std::list<char> list;
  std::views::drop(list, 0ul);
}

The constraint satisfaction failure diagnostic says:

libstdc++-v3/include/std/ranges:2100:10:   required for the satisfaction of
‘__can_drop_view<_Range, _Tp>’ [with _Range = std::__cxx11::list<char,
std::allocator<char> >&; _Tp = lon
g unsigned int]
libstdc++-v3/include/std/ranges:2101:6:   in requirements  [with _Tp = long
unsigned int; _Range = std::__cxx11::list<char, std::allocator<char> >&]
libstdc++-v3/include/std/ranges:2101:24: note: the required expression
‘drop_view<...auto...>{declval<_Range>(), declval<_Tp>()}’ is invalid, because
 2101 |           = requires { drop_view{std::declval<_Range>(),
std::declval<_Tp>()}; };
      |                       
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

but it strangely doesn't explain why the expression is invalid.  Turns out if
we pass -Wsystem-headers to the command line, then we do get an explanation in
the form of a warning:

<snip>
libstdc++-v3/include/std/ranges:2101:24: warning: narrowing conversion of
‘std::declval<long unsigned int>()’ from ‘long unsigned int’ to
‘std::ranges::range_difference_t<std::ranges::ref_view<std::__cxx11::list<char>
> >’ {aka ‘long int’} [-Wnarrowing]

So I suppose we're correct to reject the testcase, since narrowing conversions
aren't permitted in braced init lists (and views​::​drop(E, F) is specified to
be expression-equivalent to the braced init ranges​::​drop_­view{E, F}).

But it's perhaps a frontend bug that we need to pass -Wsystem-headers to get
the warning here in the first place; I'll file a PR for this issue.

[Bug libstdc++/99433] [11 Regression] custom friend pipe-operator| conflicts with range adaptor?

[gcc-bugs at marehr dot dialup.fu-berlin.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99433

--- Comment #7 from gcc-bugs at marehr dot dialup.fu-berlin.de ---
Thank you for the quick analysis!

> views​::​drop(E, F) is specified to be expression-equivalent to the braced init ranges​::​drop_­view{E, F}

Is not completely true, right? As the narrowing warning shows:

```
libstdc++-v3/include/std/ranges:2101:24: warning: narrowing conversion of
‘std::declval<long unsigned int>()’ from ‘long unsigned int’ to
‘std::ranges::range_difference_t<std::ranges::ref_view<std::__cxx11::list<char>
> >’ {aka ‘long int’} [-Wnarrowing]
```

There is some `std::views::all` involved.

But the following expressions

```
#include <list>
#include <ranges>

int main()
{
  std::list<char> list;
  // std::views::drop(list, 0ull); // does not compile
  std::ranges::drop_view{list, 0ull}; // does compile without warnings
  std::ranges::drop_view{std::views::all(list), 0ull}; // does compile without
warnings
}
```

do compile without any warnings when using `g++-11 -std=c++2a -pedantic -Wall
-Wextra`!

Even when adding `-Wsystem-headers` there is no "narrowing" warning found in
those expressions.

Thank you for your incredible help!

[Bug libstdc++/99433] [11 Regression] custom friend pipe-operator| conflicts with range adaptor?

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99433

--- Comment #8 from Patrick Palka <ppalka at gcc dot gnu.org> ---
(In reply to gcc-bugs from comment #7)
> Thank you for the quick analysis!
> 
> > views​::​drop(E, F) is specified to be expression-equivalent to the braced init ranges​::​drop_­view{E, F}
> 
> Is not completely true, right? As the narrowing warning shows:
> 
> ```
> libstdc++-v3/include/std/ranges:2101:24: warning: narrowing conversion of
> ‘std::declval<long unsigned int>()’ from ‘long unsigned int’ to
> ‘std::ranges::range_difference_t<std::ranges::ref_view<std::__cxx11::
> list<char> > >’ {aka ‘long int’} [-Wnarrowing]
> ```
> 
> There is some `std::views::all` involved.

Yeah, that comes from drop_view's deduction guide

  template<typename _Range>
    drop_view(_Range&&, range_difference_t<_Range>)
      -> drop_view<views::all_t<_Range>>;

> 
> But the following expressions
> 
> ```
> #include <list>
> #include <ranges>
> 
> int main()
> {
>   std::list<char> list;
>   // std::views::drop(list, 0ull); // does not compile
>   std::ranges::drop_view{list, 0ull}; // does compile without warnings
>   std::ranges::drop_view{std::views::all(list), 0ull}; // does compile
> without warnings
> }
> ```
> 
> do compile without any warnings when using `g++-11 -std=c++2a -pedantic
> -Wall -Wextra`!
> 
> Even when adding `-Wsystem-headers` there is no "narrowing" warning found in
> those expressions.


Ah, I think that's because 0ull is a constant expression, and a narrowing
conversion in a braced init list _is_ allowed if the value is a constant
expression that's representable in the target type.  If you replace 0ull with
some non-constant expression of the same type, the narrowing warning reappears.