[Bug libstdc++/100577] New: [11/12 Regression] Unhelpful diagnostics for ill-formed call to partially applied range adaptor object

[Bug libstdc++/100577] New: [11/12 Regression] Unhelpful diagnostics for ill-formed call to partially applied range adaptor object

[ppalka at gcc dot gnu.org]

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

            Bug ID: 100577
           Summary: [11/12 Regression] Unhelpful diagnostics for
                    ill-formed call to partially applied range adaptor
                    object
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ppalka at gcc dot gnu.org
  Target Milestone: ---

For the following invalid testcase:

#include <ranges>

int main() {
  extern int x[100];
  std::views::take(nullptr)(x);
}

GCC 10 emits the following errors:

In file included from <stdin>:1:                                                
/usr/include/c++/10/ranges: In instantiation of
‘std::ranges::views::<lambda(_Range&&, _Tp&&)> [with _Range = int (&)[100]; _Tp
= std::nullptr_t]’:                                                             
/usr/include/c++/10/ranges:1142:27:   required from
‘std::ranges::views::__adaptor::_RangeAdaptor<_Callable>::operator()<{std::nullptr_t}>::<lambda(_Range&&)>
[with _Range = int (&)[100]]’                       
/usr/include/c++/10/ranges:1162:44:   required from ‘constexpr auto
std::ranges::views::__adaptor::_RangeAdaptorClosure<_Callable>::operator()(_Range&&)
const [with _Range = int (&)[100]; _Callable = std::ranges
::views::__adaptor::_RangeAdaptor<_Callable>::operator()<{std::nullptr_t}>::<lambda(_Range&&)>]’ 
<stdin>:5:30:   required from here                                              
/usr/include/c++/10/ranges:2148:68: error: class template argument deduction
failed:                                                                         
 2148 |  return take_view{std::forward<_Range>(__r), std::forward<_Tp>(__n)};   
      |                                                                    ^    
/usr/include/c++/10/ranges:2148:68: error: no matching function for call to
‘take_view(int [100], std::nullptr_t)’                                         
                                                       [...]
/usr/include/c++/10/ranges:2140:5: note: candidate:
‘std::ranges::take_view(_Range&&, std::ranges::range_difference_t<_Range>)->
std::ranges::take_view<std::ranges::views::all_t<_Range> > [with _Range = int
(&)[
100]; std::ranges::views::all_t<_Range> = std::ranges::ref_view<int [100]>;
std::ranges::range_difference_t<_Range> = long int]’                            
 2140 |     take_view(_Range&&, range_difference_t<_Range>)                     
      |     ^~~~~~~~~                                                           


Whereas GCC 11, after r11-8053, emits an almost useless error:

<stdin>:5:28: error: use of deleted function ‘constexpr auto
std::ranges::views::__adaptor::_Partial<_Adaptor, _Arg>::operator()(_Range&&)
const && [with _Range = int (&)[100]; _Adaptor = std::ranges::views::_Take;
_Arg = std::nullptr_t]’
In file included from <stdin>:1:
/home/patrick/code/gcc-master/libstdc++-v3/include/std/ranges:865:9: note:
declared here
  865 |         operator()(_Range&& __r) const && = delete;
      |         ^~~~~~~~

[Bug libstdc++/100577] [11/12 Regression] Unhelpful diagnostics for ill-formed call to partially applied range adaptor object

[ppalka at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |10.1.0, 10.2.0, 10.3.0
           Keywords|                            |diagnostic
   Target Milestone|---                         |11.2
      Known to fail|                            |11.1.0

--- Comment #1 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Diagnostics for ill-formed calls to adaptor pipelines regress similarly:

#include <ranges>

int main() {
  extern int x[100];
  (std::views::join | std::views::join)(x);
}

<stdin>: In function ‘int main()’:
<stdin>:5:40: error: use of deleted function ‘constexpr auto
std::ranges::views::__adaptor::_Pipe<_Lhs, _Rhs>::operator()(_Range&&) const &&
[with _Range = int (&)[100]; _Lhs = std::ranges::views::_Join; _Rhs = 
std::ranges::views::_Join]’
In file included from <stdin>:1:
/home/patrick/code/gcc-master/libstdc++-v3/include/std/ranges:901:9: note:
declared here
  901 |         operator()(_Range&& __r) const && = delete;
      |         ^~~~~~~~

[Bug libstdc++/100577] [11/12 Regression] Unhelpful diagnostics for ill-formed call to partially applied range adaptor object

[ppalka at gcc dot gnu.org]

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

--- Comment #2 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Some context: the purpose of this deleted operator() is so that when invoking
an adaptor object as an rvalue, we don't ever select the const&-qualified
operator() overload, because doing so would break the perfect forwarding call
wrapper semantics required of partially applied adaptor objects and pipelines,
and cause us to silently accept the P2281 example 'x |
views::split(non_view_range)' (forming a split_view containing a dangling
reference).  This deleted const&&-qualified overload always wins over the
const&-qualified overload in the rvalue case, and so prevents this breakage
from happening.

This PR arguably calls for an improvement on the frontend side -- the
diagnostic for when overload resolution chooses a deleted function could also
perhaps explain why the other candidates weren't selected.

But I think we can also work around this deficiency on the library side by
essentially disabling the perfect forwarding call wrapper machinery for range
adaptors that don't need it, which currently is all of them except for
views::split.  For the other adaptors, we could get away with just a single
overload for  _Partial::operator() and _Pipe::operator().

[Bug libstdc++/100577] [11/12 Regression] Unhelpful diagnostics for ill-formed call to partially applied range adaptor object

[ppalka at gcc dot gnu.org]

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

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

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

--- Comment #3 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Created attachment 50804
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50804&action=edit
wip library-side patch

The attached WIP patch implements the library-side workaround mentioned in
comment #2.  With it, we recover the same diagnostics that GCC 10 gave us,
though we need to pass -fconcepts-diagnostics-depth=3 to see them.

[Bug libstdc++/100577] [11/12 Regression] Unhelpful diagnostics for ill-formed call to partially applied range adaptor object

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 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:57ed620ebfa4275d04efeec973e88f506b0e3ba8

commit r12-1184-g57ed620ebfa4275d04efeec973e88f506b0e3ba8
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.

[Bug libstdc++/100577] [11/12 Regression] Unhelpful diagnostics for ill-formed call to partially applied range adaptor object

[cvs-commit at gcc dot gnu.org]

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

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

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)

[Bug libstdc++/100577] [11/12 Regression] Unhelpful diagnostics for ill-formed call to partially applied range adaptor object

[ppalka at gcc dot gnu.org]

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

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

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

--- Comment #6 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Fixed for GCC 11.2/12