[Bug libstdc++/100824] New: ranges::size should treat the subexpression as an lvalue

[Bug libstdc++/100824] New: ranges::size should treat the subexpression as an lvalue

[hewillk at gmail dot com]

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

            Bug ID: 100824
           Summary: ranges::size should treat the subexpression as an
                    lvalue
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: hewillk at gmail dot com
  Target Milestone: ---

The CPO described in [range.access] treats the subexpression E as an lvalue,
but ranges::size, ranges::empty, and ranges::data forward E perfectly, which
makes the following case behavior wrong:

https://godbolt.org/z/7KoT3WE41

#include <ranges>
#include <array>

struct R : std::ranges::view_base {
  constexpr int* begin() & { return nullptr; }
  constexpr int* end() & { return nullptr; }
  constexpr int size() & { return 42; }
};
template <>
inline constexpr bool std::ranges::enable_borrowed_range<R> = true;

// call ranges::end(e) - ranges::begin(e) incorrectly
static_assert(std::ranges::size(R{}));

int main() {
  std::array r{42, 42, 42};
  // call e.data() correctly
  (r | std::views::all).data();

  // call std::to_address(ranges::begin(e)) incorrectly
  std::ranges::data(r | std::views::all);
}

[Bug libstdc++/100824] ranges::size should treat the subexpression as an lvalue

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

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

[Bug libstdc++/100824] ranges::size should treat the subexpression as an lvalue

[redi at gcc dot gnu.org]

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

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I didn't completely implement p2091r0 which changed the value categories that
these CPOs are expression-equivalent to.

[Bug libstdc++/100824] ranges::size should treat the subexpression as an lvalue

[cvs-commit at gcc dot gnu.org]

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

--- Comment #2 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

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

commit r12-1214-gee9548b36a7f17e8a63585b58f340c93dcba95d8
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jun 4 15:59:38 2021 +0100

    libstdc++: Fix value categories used by ranges access CPOs [PR 100824]

    The implementation of P2091R0 was incomplete, so that some range access
    CPOs used perfect forwarding where they should not. This fixes it by
    consistently operating on lvalues.

    Some additional changes that are not necessary to fix the bug:

    Modify the __as_const helper to simplify its usage. Instead of deducing
    the value category from its argument, and requiring callers to forward
    the argument as the correct category, add a non-deduced template
    parameter which is used for the value category and accept the argument
    as an lvalue. This means callers say __as_const<T>(t) instead of
    __as_const(std::forward<T>(t)).

    Always use an lvalue reference type as the template argument for the
    _S_noexcept helpers, so that we only instantiate one specialization for
    lvalues and rvalues of the same type.

    Move some helper concepts and functions from namespace std::__detail
    to ranges::__cust_access, to be consistent with the ranges::begin CPO.
    This ensures that the __adl_begin concept and the _Begin::operator()
    function are in the same namespace, so unqualified lookup is consistent
    and the poison pills for begin are visible to both.

    Simplified static assertions for arrays, because the expression a+0 is
    already ill-formed for an array of incomplete type.

    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

    libstdc++-v3/ChangeLog:

            PR libstdc++/100824
            * include/bits/iterator_concepts.h (__detail::__decay_copy)
            (__detail::__member_begin, __detail::__adl_begin): Move to
            namespace ranges::__cust_access.
            (__detail::__ranges_begin): Likewise, and rename to __begin.
            Remove redundant static assertion.
            * include/bits/ranges_base.h (_Begin, _End, _RBegin, _REnd):
            Use lvalue in noexcept specifier.
            (__as_const): Add non-deduced parameter for value category.
            (_CBegin, _CEnd, _CRBegin, _CREnd, _CData): Adjust uses of
            __as_const.
            (__member_size, __adl_size, __member_empty, __size0_empty):
            (__eq_iter_empty, __adl_data): Use lvalue objects in
            requirements.
            (__sentinel_size): Likewise. Add check for conversion to
            unsigned-like.
            (__member_data): Allow non-lvalue types to satisfy the concept,
            but use lvalue object in requirements.
            (_Size, _SSize): Remove forwarding to always use an lvalue.
            (_Data): Likewise. Add static assertion for arrays.
            * testsuite/std/ranges/access/cdata.cc: Adjust expected
            behaviour for rvalues. Add negative tests for ill-formed
            expressions.
            * testsuite/std/ranges/access/data.cc: Likewise.
            * testsuite/std/ranges/access/empty.cc: Adjust expected
            behaviour for rvalues.
            * testsuite/std/ranges/access/size.cc: Likewise.

[Bug libstdc++/100824] ranges::size should treat the subexpression as an lvalue

[hewillk at gmail dot com]

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

--- Comment #3 from 康桓瑋 <hewillk at gmail dot com> ---
Another trivial issue is that LWG 3403 is not implemented and the
ranges​::​data is missing the decay-copy part, making it unable to work with
the following type.

struct A { int*&& data(); };

[Bug libstdc++/100824] ranges::size should treat the subexpression as an lvalue

[redi at gcc dot gnu.org]

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

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Ah yes, the __member_data and __adl_data concepts aren't decaying the result
types.

[Bug libstdc++/100824] ranges::size should treat the subexpression as an lvalue

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

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

commit r12-1227-g3e5f2425f80aedd00f28235022a2755eb46f310d
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jun 4 20:25:39 2021 +0100

    libstdc++: Fix helper concept for ranges::data [PR 100824]

    We need to decay the result of t.data() before checking if it's a
    pointer.

    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

    libstdc++-v3/ChangeLog:

            PR libstdc++/100824
            * include/bits/ranges_base.h (__member_data): Use __decay_copy.
            * testsuite/std/ranges/access/data.cc: Add testcase from PR.

[Bug libstdc++/100824] ranges::size should treat the subexpression as an lvalue

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
And LWG 3403 was fixed by r12-1228.

I plan to backport these three patches too.

Thanks for all the bug reports like this!

[Bug libstdc++/100824] ranges::size should treat the subexpression as an lvalue

[hewillk at gmail dot com]

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

--- Comment #7 from 康桓瑋 <hewillk at gmail dot com> ---
(In reply to Jonathan Wakely from comment #6)
> And LWG 3403 was fixed by r12-1228.

Hey, Jonathan, thank you for your contribution to gcc. Regarding the
implementation of LWG 3403, the return type of __int128 is unsigned. I don't
know if this is a typo.

[Bug libstdc++/100824] ranges::size should treat the subexpression as an lvalue

[redi at gcc dot gnu.org]

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

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Doh yes, that's just a brain fart

[Bug libstdc++/100824] ranges::size should treat the subexpression as an lvalue

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:

https://gcc.gnu.org/g:96963713f6a648a0ed890450e02ebdd8ff583b14

commit r12-1232-g96963713f6a648a0ed890450e02ebdd8ff583b14
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Sat Jun 5 11:42:01 2021 +0100

    libstdc++: Fix return type of ranges::ssize for 128-bit integer [PR 100824]

    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

    libstdc++-v3/ChangeLog:

            PR libstdc++/100824
            * include/bits/ranges_base.h (_SSize): Return signed type.
            * testsuite/std/ranges/access/ssize.cc: Check with __int128.

[Bug libstdc++/100824] ranges::size should treat the subexpression as an lvalue

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

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

commit r11-8555-ga842cb9c255f66f38ddd36b10c60920ed5f189de
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jun 4 15:59:38 2021 +0100

    libstdc++: Fix value categories used by ranges access CPOs [PR 100824]

    The implementation of P2091R0 was incomplete, so that some range access
    CPOs used perfect forwarding where they should not. This fixes it by
    consistently operating on lvalues.

    Some additional changes that are not necessary to fix the bug:

    Modify the __as_const helper to simplify its usage. Instead of deducing
    the value category from its argument, and requiring callers to forward
    the argument as the correct category, add a non-deduced template
    parameter which is used for the value category and accept the argument
    as an lvalue. This means callers say __as_const<T>(t) instead of
    __as_const(std::forward<T>(t)).

    Always use an lvalue reference type as the template argument for the
    _S_noexcept helpers, so that we only instantiate one specialization for
    lvalues and rvalues of the same type.

    Move some helper concepts and functions from namespace std::__detail
    to ranges::__cust_access, to be consistent with the ranges::begin CPO.
    This ensures that the __adl_begin concept and the _Begin::operator()
    function are in the same namespace, so unqualified lookup is consistent
    and the poison pills for begin are visible to both.

    Simplified static assertions for arrays, because the expression a+0 is
    already ill-formed for an array of incomplete type.

    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

    libstdc++-v3/ChangeLog:

            PR libstdc++/100824
            * include/bits/iterator_concepts.h (__detail::__decay_copy)
            (__detail::__member_begin, __detail::__adl_begin): Move to
            namespace ranges::__cust_access.
            (__detail::__ranges_begin): Likewise, and rename to __begin.
            Remove redundant static assertion.
            * include/bits/ranges_base.h (_Begin, _End, _RBegin, _REnd):
            Use lvalue in noexcept specifier.
            (__as_const): Add non-deduced parameter for value category.
            (_CBegin, _CEnd, _CRBegin, _CREnd, _CData): Adjust uses of
            __as_const.
            (__member_size, __adl_size, __member_empty, __size0_empty):
            (__eq_iter_empty, __adl_data): Use lvalue objects in
            requirements.
            (__sentinel_size): Likewise. Add check for conversion to
            unsigned-like.
            (__member_data): Allow non-lvalue types to satisfy the concept,
            but use lvalue object in requirements.
            (_Size, _SSize): Remove forwarding to always use an lvalue.
            (_Data): Likewise. Add static assertion for arrays.
            * testsuite/std/ranges/access/cdata.cc: Adjust expected
            behaviour for rvalues. Add negative tests for ill-formed
            expressions.
            * testsuite/std/ranges/access/data.cc: Likewise.
            * testsuite/std/ranges/access/empty.cc: Adjust expected
            behaviour for rvalues.
            * testsuite/std/ranges/access/size.cc: Likewise.

    (cherry picked from commit ee9548b36a7f17e8a63585b58f340c93dcba95d8)

[Bug libstdc++/100824] ranges::size should treat the subexpression as an lvalue

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:950535915868cd0b7f3dbad4321de913f01bddfc

commit r11-8556-g950535915868cd0b7f3dbad4321de913f01bddfc
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jun 4 20:25:39 2021 +0100

    libstdc++: Fix helper concept for ranges::data [PR 100824]

    We need to decay the result of t.data() before checking if it's a
    pointer.

    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

    libstdc++-v3/ChangeLog:

            PR libstdc++/100824
            * include/bits/ranges_base.h (__member_data): Use __decay_copy.
            * testsuite/std/ranges/access/data.cc: Add testcase from PR.

    (cherry picked from commit 3e5f2425f80aedd00f28235022a2755eb46f310d)

[Bug libstdc++/100824] ranges::size should treat the subexpression as an lvalue

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jonathan Wakely
<redi@gcc.gnu.org>:

https://gcc.gnu.org/g:2ba1680d3e9f3bb99783519a08eed6435f131045

commit r11-8558-g2ba1680d3e9f3bb99783519a08eed6435f131045
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Sat Jun 5 11:42:01 2021 +0100

    libstdc++: Fix return type of ranges::ssize for 128-bit integer [PR 100824]

    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

    libstdc++-v3/ChangeLog:

            PR libstdc++/100824
            * include/bits/ranges_base.h (_SSize): Return signed type.
            * testsuite/std/ranges/access/ssize.cc: Check with __int128.

    (cherry picked from commit 96963713f6a648a0ed890450e02ebdd8ff583b14)

[Bug libstdc++/100824] ranges::size should treat the subexpression as an lvalue

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.2

--- Comment #13 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I'm not going to backport this to gcc-10, so FIXED.

[Bug libstdc++/100824] ranges::size should treat the subexpression as an lvalue

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

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

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> ---
.