Bug 95322 - std::list | take | transform, expression does not work cbegin() == end()
Summary: std::list | take | transform, expression does not work cbegin() == end()
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 10.1.1
: P3 normal
Target Milestone: 10.2
Assignee: Jonathan Wakely
URL:
Keywords: rejects-valid
Depends on:
Blocks:
 
Reported: 2020-05-25 18:16 UTC by gcc-bugs
Modified: 2020-12-01 16:20 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2020-05-26 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description gcc-bugs 2020-05-25 18:16:15 UTC
Hello gcc-team,

I'm not sure if this is intended behaviour, or if this is a defect in the standard, or something that you should never write in a generic context.

ASFAIK you can use that expression on any container and on any view, but not on some(?!) combinations of views? This is a bit unexpected.

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

int main()
{

    std::list v{0,1 ,2, 3, 4, 5, 5, 6}; // works if std::vector

    auto view1 = v | std::views::take(5);
    auto view2 = v | std::views::transform([] (int i) { return i + 3;});
    auto view3 = view1 | std::views::transform([] (int i) { return i + 3;});
    
    std::ranges::cbegin(view1) == std::ranges::end(view1); // works
    std::ranges::cbegin(view2) == std::ranges::end(view2); // works
    std::ranges::cbegin(view3) == std::ranges::end(view3); // does not work
    return 0;
}
```

https://godbolt.org/z/4Nzr69
Comment 1 Jonathan Wakely 2020-05-26 13:13:50 UTC
I think it's a defect in the standard, the sentinel for transform_view can only be compared to iterators of the same constness:

	  friend constexpr bool
	  operator==(const _Iterator<_Const>& __x, const _Sentinel& __y)
	  { return __y.__equal(__x); }

I think it should work though.
Comment 2 Jonathan Wakely 2020-05-26 13:18:33 UTC
There's an implicit conversion from sentinel<false> to sentinel<true>, but that would only happen if the operator== had been found, and since it's a hidden friend it can't be found unless one of the operands is already sentinel<true>.
Comment 3 Jonathan Wakely 2020-05-26 13:21:56 UTC
Adding this to transform_view::_Sentinel fixes it:

	  friend constexpr bool
	  operator==(const _Iterator<true>& __x, const _Sentinel& __y)
	  requires (!_Const)
	  { return _Sentinel<true>(__y).__equal(__x); }
Comment 4 Patrick Palka 2020-05-26 13:54:51 UTC
Attempting to work around this by using views::common like so

       auto view3 = view1 | std::views::transform([] (int i) { return i + 3;}) | std::views::common;
       /* ... */
       std::ranges::cbegin(view3) == std::ranges::end(view3);

instead results is a compilation failure in stl_iterator.h:

/home/patrick/code/gcc/libstdc++-v3/include/bits/stl_iterator.h:1772:27: error: ‘_Common_iter_proxy’ was not declared in this scope; did you mean ‘std::__detail::_Common_iter_proxy’?         1772 |  return _Common_iter_proxy(*_M_it);                                                    
      |         ~~~~~~~~~~~~~~~~~~^~~~~~~~                                                                                                                                                          |         std::__detail::_Common_iter_proxy                                                                                                                                             /home/patrick/code/gcc/libstdc++-v3/include/bits/stl_iterator.h:1572:13: note: ‘std::__detail::_Common_iter_proxy’ declared here
Comment 5 GCC Commits 2020-05-26 20:45:06 UTC
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:3bf5e7657b752cc2352778e8c20ac9cdddca4f93

commit r11-645-g3bf5e7657b752cc2352778e8c20ac9cdddca4f93
Author: Patrick Palka <ppalka@redhat.com>
Date:   Tue May 26 16:17:34 2020 -0400

    libstdc++: Fix common_iterator::operator-> [PR95322]
    
    This patch fixes the definition of common_iterator::operator-> when the
    underlying iterator's operator* returns a non-reference.
    
    The first problem is that the class __detail::_Common_iter_proxy is used
    unqualified.  Fixing that revealed another problem: the class's template
    friend declaration of common_iterator doesn't match up with the
    definition of common_iterator, because the friend declaration isn't
    constrained.
    
    If we try to make the friend declaration match up by adding constraints,
    we run into frontend bug PR93467.  So we currently can't correctly
    express this friend relation between __detail::_Common_iter_proxy and
    common_iterator.
    
    As a workaround to this frontend bug, this patch moves the definition of
    _Common_iter_proxy into the class template of common_iterator so that we
    could instead express the friend relation via the injected-class-name.
    
    (This bug was found when attempting to use views::common to work around
    the compile failure with the testcase in PR95322.)
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/95322
            * include/bits/stl_iterator.h (__detail::_Common_iter_proxy):
            Remove and instead define it ...
            (common_iterator::_Proxy): ... here.
            (common_iterator::operator->): Use it.
            * testsuite/24_iterators/common_iterator/2.cc: New test.
            * testsuite/std/ranges/adaptors/95322.cc: New test.
Comment 6 Jonathan Wakely 2020-05-27 20:00:33 UTC
A new LWG issue has been submitted, and there is a suggested resolution.
Comment 7 Daniel Krügler 2020-05-27 20:04:50 UTC
(In reply to Jonathan Wakely from comment #6)
> A new LWG issue has been submitted, and there is a suggested resolution.

Will take care and inform in this issue here.
Comment 8 Jonathan Wakely 2020-05-27 20:29:48 UTC
Thanks Daniel, I'm just testing Casey's resolution, and extending it to join_view, so I'll send another mail once that testing is complete.
Comment 9 GCC Commits 2020-05-27 21:08:23 UTC
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:6c2582c0406250c66e2eb3651f8e8638796b7f53

commit r11-673-g6c2582c0406250c66e2eb3651f8e8638796b7f53
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed May 27 22:08:15 2020 +0100

    libstdc++: Fix view adaptors for mixed-const sentinels and iterators (PR 95322)
    
    The bug report is that transform_view's sentinel<false> cannot be
    compared to its iterator<true>.  The comparison is supposed to use
    operator==(iterator<Const>, sentinel<Const>) after converting
    sentinel<false> to sentinel<true>. However, the operator== is a hidden
    friend so is not a candidate when comparing iterator<true> with
    sentinel<false>. The required conversion would only happen if we'd found
    the operator, but we can't find the operator until after the conversion
    happens.
    
    A new LWG issue has been reported, but not yet assigned a number.  The
    solution suggested by Casey Carter is to make the hidden friends of the
    sentinel types work with iterators of any const-ness, so that no
    conversions are required.
    
    Patrick Palka observed that join_view has a similar problem and a
    similar fix is used for its sentinel.
    
            PR libstdc++/95322
            * include/std/ranges (transform_view::_Sentinel): Allow hidden
            friends to work with _Iterator<true> and _Iterator<false>.
            (join_view::_Sentinel): Likewise.
            * testsuite/std/ranges/adaptors/95322.cc: New test.
Comment 10 Jonathan Wakely 2020-05-27 21:11:35 UTC
Fixed on master so far, backport to follow soon.
Comment 11 GCC Commits 2020-05-28 01:48:25 UTC
The releases/gcc-10 branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

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

commit r10-8194-gfc78e991c35a5ee14efafb4e5566a9570fa31dd4
Author: Patrick Palka <ppalka@redhat.com>
Date:   Tue May 26 16:17:34 2020 -0400

    libstdc++: Fix common_iterator::operator-> [PR95322]
    
    This patch fixes the definition of common_iterator::operator-> when the
    underlying iterator's operator* returns a non-reference.
    
    The first problem is that the class __detail::_Common_iter_proxy is used
    unqualified.  Fixing that revealed another problem: the class's template
    friend declaration of common_iterator doesn't match up with the
    definition of common_iterator, because the friend declaration isn't
    constrained.
    
    If we try to make the friend declaration match up by adding constraints,
    we run into frontend bug PR93467.  So we currently can't correctly
    express this friend relation between __detail::_Common_iter_proxy and
    common_iterator.
    
    As a workaround to this frontend bug, this patch moves the definition of
    _Common_iter_proxy into the class template of common_iterator so that we
    could instead express the friend relation via the injected-class-name.
    
    (This bug was found when attempting to use views::common to work around
    the compile failure with the testcase in PR95322.)
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/95322
            * include/bits/stl_iterator.h (__detail::_Common_iter_proxy):
            Remove and instead define it ...
            (common_iterator::_Proxy): ... here.
            (common_iterator::operator->): Use it.
            * testsuite/24_iterators/common_iterator/2.cc: New test.
            * testsuite/std/ranges/adaptors/95322.cc: New test.
    
    (cherry picked from commit 3bf5e7657b752cc2352778e8c20ac9cdddca4f93)
Comment 12 Daniel Krügler 2020-05-28 18:51:52 UTC
(In reply to Daniel Krügler from comment #7)
> (In reply to Jonathan Wakely from comment #6)
> > A new LWG issue has been submitted, and there is a suggested resolution.
> 
> Will take care and inform in this issue here.

The new LWG issue exists now:

https://cplusplus.github.io/LWG/issue3448
Comment 13 GCC Commits 2020-07-09 12:59:24 UTC
The releases/gcc-10 branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:38250e577e26de7aace65b4d32a94a1404f076a9

commit r10-8450-g38250e577e26de7aace65b4d32a94a1404f076a9
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed May 27 22:08:15 2020 +0100

    libstdc++: Fix view adaptors for mixed-const sentinels and iterators (PR 95322)
    
    The bug report is that transform_view's sentinel<false> cannot be
    compared to its iterator<true>.  The comparison is supposed to use
    operator==(iterator<Const>, sentinel<Const>) after converting
    sentinel<false> to sentinel<true>. However, the operator== is a hidden
    friend so is not a candidate when comparing iterator<true> with
    sentinel<false>. The required conversion would only happen if we'd found
    the operator, but we can't find the operator until after the conversion
    happens.
    
    A new LWG issue has been reported, but not yet assigned a number.  The
    solution suggested by Casey Carter is to make the hidden friends of the
    sentinel types work with iterators of any const-ness, so that no
    conversions are required.
    
    Patrick Palka observed that join_view has a similar problem and a
    similar fix is used for its sentinel.
    
            PR libstdc++/95322
            * include/std/ranges (transform_view::_Sentinel): Allow hidden
            friends to work with _Iterator<true> and _Iterator<false>.
            (join_view::_Sentinel): Likewise.
            * testsuite/std/ranges/adaptors/95322.cc: New test.
    
    (cherry picked from commit 6c2582c0406250c66e2eb3651f8e8638796b7f53)
Comment 14 Jonathan Wakely 2020-07-09 14:51:21 UTC
fixed for 10.2
Comment 15 gcc-bugs 2020-07-09 17:54:43 UTC
Thank you!
Comment 16 GCC Commits 2020-08-27 01:53:40 UTC
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:3ae0cd94abc15e33dc06ca7a5f76f14b1d74129f

commit r11-2898-g3ae0cd94abc15e33dc06ca7a5f76f14b1d74129f
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Aug 26 21:51:48 2020 -0400

    libstdc++: Implement remaining piece of LWG 3448
    
    Almost all of the proposed resolution for LWG 3448 is already
    implemented; the only part left is to adjust the return type of
    transform_view::sentinel::operator-.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/95322
            * include/std/ranges (transform_view::sentinel::__distance_from):
            Give this a deduced return type.
            (transform_view::sentinel::operator-): Adjust the return type so
            that it's based on the constness of the iterator rather than
            that of the sentinel.
            * testsuite/std/ranges/adaptors/95322.cc: Refer to LWG 3488.
Comment 17 GCC Commits 2020-10-12 17:48:27 UTC
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

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

commit r11-3834-ge066821b6f6b7332c7a67981f7b33c9ba0ccaee7
Author: Patrick Palka <ppalka@redhat.com>
Date:   Mon Oct 12 13:46:21 2020 -0400

    libstdc++: Apply proposed resolution for LWG 3449 [PR95322]
    
    Now that the frontend bug PR96805 is fixed, we can cleanly apply the
    proposed resolution for this issue.
    
    This slightly deviates from the proposed resolution by declaring _CI a
    member of take_view instead of take_view::_Sentinel, since it doesn't
    depend on anything within _Sentinel anymore.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/95322
            * include/std/ranges (take_view::_CI): Define this alias
            template as per LWG 3449 and remove ...
            (take_view::_Sentinel::_CI): ... this type alias.
            (take_view::_Sentinel::operator==): Adjust use of _CI
            accordingly.  Define a second overload that accepts an iterator
            of the opposite constness as per LWG 3449.
            (take_while_view::_Sentinel::operator==): Likewise.
            * testsuite/std/ranges/adaptors/95322.cc: Add tests for LWG 3449.
Comment 18 GCC Commits 2020-10-21 01:57:38 UTC
The releases/gcc-10 branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:574ab3c85bb393e0ed0171b96eb42e0dd1e91de4

commit r10-8927-g574ab3c85bb393e0ed0171b96eb42e0dd1e91de4
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Aug 26 21:51:48 2020 -0400

    libstdc++: Implement remaining piece of LWG 3448
    
    Almost all of the proposed resolution for LWG 3448 is already
    implemented; the only part left is to adjust the return type of
    transform_view::sentinel::operator-.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/95322
            * include/std/ranges (transform_view::sentinel::__distance_from):
            Give this a deduced return type.
            (transform_view::sentinel::operator-): Adjust the return type so
            that it's based on the constness of the iterator rather than
            that of the sentinel.
            * testsuite/std/ranges/adaptors/95322.cc: Refer to LWG 3488.
    
    (cherry picked from commit 3ae0cd94abc15e33dc06ca7a5f76f14b1d74129f)
Comment 19 GCC Commits 2020-12-01 16:20:48 UTC
The releases/gcc-10 branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

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

commit r10-9104-gbe5f22ebe964244223f178330a66b67d3f58334e
Author: Patrick Palka <ppalka@redhat.com>
Date:   Mon Oct 12 13:46:21 2020 -0400

    libstdc++: Apply proposed resolution for LWG 3449 [PR95322]
    
    Now that the frontend bug PR96805 is fixed, we can cleanly apply the
    proposed resolution for this issue.
    
    This slightly deviates from the proposed resolution by declaring _CI a
    member of take_view instead of take_view::_Sentinel, since it doesn't
    depend on anything within _Sentinel anymore.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/95322
            * include/std/ranges (take_view::_CI): Define this alias
            template as per LWG 3449 and remove ...
            (take_view::_Sentinel::_CI): ... this type alias.
            (take_view::_Sentinel::operator==): Adjust use of _CI
            accordingly.  Define a second overload that accepts an iterator
            of the opposite constness as per LWG 3449.
            (take_while_view::_Sentinel::operator==): Likewise.
            * testsuite/std/ranges/adaptors/95322.cc: Add tests for LWG 3449.
    
    (cherry picked from commit e066821b6f6b7332c7a67981f7b33c9ba0ccaee7)