Skip to content

Fix handling of array rvalues for ranges::cbegin and its friends #3316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jan 12, 2023
Merged

Fix handling of array rvalues for ranges::cbegin and its friends #3316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
24d6419
Test coverage for array rvalues of incomplete elements
frederick-vs-ja Dec 31, 2022
69135fd
Fix handling of array rvalues for some CPOs
frederick-vs-ja Dec 31, 2022
d1505ad
Workaround for clang
frederick-vs-ja Dec 31, 2022
1180750
Clang-format
frederick-vs-ja Dec 31, 2022
006f552
Pass lvalues to `begin` etc.
frederick-vs-ja Jan 1, 2023
8b32947
Fix copy pasta: use `const_sentinel` instead of `const_iterator`
frederick-vs-ja Jan 1, 2023
6493d8e
Fix copy pasta: use `_Rend_adapted`
frederick-vs-ja Jan 1, 2023
997f655
The same treatment should be done for `cdata`
frederick-vs-ja Jan 1, 2023
e0af83f
Name changes and transition comments by @strega-nil-ms
frederick-vs-ja Jan 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 66 additions & 30 deletions stl/inc/xutility
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2479,17 +2479,34 @@ namespace ranges {
_NODISCARD constexpr auto _As_const_pointer(const _Ty* _Ptr) noexcept {
return _Ptr;
}

template <class _Ty>
using _Begin_on_const = decltype(_RANGES begin(_RANGES _Possibly_const_range(_STD declval<_Ty&>())));

// TRANSITION, LLVM-55945
template <class _Ty>
concept _Range_accessible_and_begin_adaptable =
_Should_range_access<_Ty>
&& requires(
_Ty& _Val) { const_iterator<_Begin_on_const<_Ty>>{_RANGES begin(_RANGES _Possibly_const_range(_Val))}; };

template <class _Ty>
using _End_on_const = decltype(_RANGES end(_RANGES _Possibly_const_range(_STD declval<_Ty&>())));

// TRANSITION, LLVM-55945
template <class _Ty>
concept _Range_accessible_and_end_adaptable =
_Should_range_access<_Ty>
&& requires(
_Ty& _Val) { const_sentinel<_End_on_const<_Ty>>{_RANGES end(_RANGES _Possibly_const_range(_Val))}; };
#endif // _HAS_CXX23

struct _Cbegin_fn {
#if _HAS_CXX23
template <_Should_range_access _Ty,
class _Uty = decltype(_RANGES begin(_RANGES _Possibly_const_range(_STD declval<_Ty&>())))>
_NODISCARD constexpr auto operator()(_Ty&& _Val) const
noexcept(noexcept(const_iterator<_Uty>{_RANGES begin(_RANGES _Possibly_const_range(_Val))}))
requires requires { const_iterator<_Uty>{_RANGES begin(_RANGES _Possibly_const_range(_Val))}; }
{
return const_iterator<_Uty>{_RANGES begin(_RANGES _Possibly_const_range(_Val))};
template <_Range_accessible_and_begin_adaptable _Ty>
_NODISCARD constexpr auto operator()(_Ty&& _Val) const noexcept(
noexcept(const_iterator<_Begin_on_const<_Ty>>{_RANGES begin(_RANGES _Possibly_const_range(_Val))})) {
return const_iterator<_Begin_on_const<_Ty>>{_RANGES begin(_RANGES _Possibly_const_range(_Val))};
}
#else // ^^^ C++23 / C++20 vvv
template <class _Ty, class _CTy = _Const_thru_ref<_Ty>>
Expand All @@ -2508,13 +2525,10 @@ namespace ranges {

struct _Cend_fn {
#if _HAS_CXX23
template <_Should_range_access _Ty,
class _Uty = decltype(_RANGES end(_RANGES _Possibly_const_range(_STD declval<_Ty&>())))>
template <_Range_accessible_and_end_adaptable _Ty>
_NODISCARD constexpr auto operator()(_Ty&& _Val) const
noexcept(noexcept(const_sentinel<_Uty>{_RANGES end(_RANGES _Possibly_const_range(_Val))}))
requires requires { const_sentinel<_Uty>{_RANGES end(_RANGES _Possibly_const_range(_Val))}; }
{
return const_sentinel<_Uty>{_RANGES end(_RANGES _Possibly_const_range(_Val))};
noexcept(noexcept(const_sentinel<_End_on_const<_Ty>>{_RANGES end(_RANGES _Possibly_const_range(_Val))})) {
return const_sentinel<_End_on_const<_Ty>>{_RANGES end(_RANGES _Possibly_const_range(_Val))};
}
#else // ^^^ C++23 / C++20 vvv
template <class _Ty, class _CTy = _Const_thru_ref<_Ty>>
Expand Down Expand Up @@ -2670,15 +2684,34 @@ namespace ranges {
_EXPORT_STD inline constexpr _Rend::_Cpo rend;
}

#if _HAS_CXX23
template <class _Ty>
using _Rbegin_on_const = decltype(_RANGES rbegin(_RANGES _Possibly_const_range(_STD declval<_Ty&>())));

// TRANSITION, LLVM-55945
template <class _Ty>
concept _Range_accessible_and_rbegin_adaptable =
_Should_range_access<_Ty>
&& requires(
_Ty& _Val) { const_iterator<_Rbegin_on_const<_Ty>>{_RANGES rbegin(_RANGES _Possibly_const_range(_Val))}; };

template <class _Ty>
using _Rend_on_const = decltype(_RANGES rend(_RANGES _Possibly_const_range(_STD declval<_Ty&>())));

// TRANSITION, LLVM-55945
template <class _Ty>
concept _Range_accessible_and_rend_adaptable =
_Should_range_access<_Ty>
&& requires(
_Ty& _Val) { const_sentinel<_Rend_on_const<_Ty>>{_RANGES rend(_RANGES _Possibly_const_range(_Val))}; };
#endif // _HAS_CXX23

struct _Crbegin_fn {
#if _HAS_CXX23
template <_Should_range_access _Ty,
class _Uty = decltype(_RANGES rbegin(_RANGES _Possibly_const_range(_STD declval<_Ty&>())))>
_NODISCARD constexpr auto operator()(_Ty&& _Val) const
noexcept(noexcept(const_iterator<_Uty>{_RANGES rbegin(_RANGES _Possibly_const_range(_Val))}))
requires requires { const_iterator<_Uty>{_RANGES rbegin(_RANGES _Possibly_const_range(_Val))}; }
{
return const_iterator<_Uty>{_RANGES rbegin(_RANGES _Possibly_const_range(_Val))};
template <_Range_accessible_and_rbegin_adaptable _Ty>
_NODISCARD constexpr auto operator()(_Ty&& _Val) const noexcept(
noexcept(const_iterator<_Rbegin_on_const<_Ty>>{_RANGES rbegin(_RANGES _Possibly_const_range(_Val))})) {
return const_iterator<_Rbegin_on_const<_Ty>>{_RANGES rbegin(_RANGES _Possibly_const_range(_Val))};
}
#else // ^^^ C++23 / C++20 vvv
template <class _Ty, class _CTy = _Const_thru_ref<_Ty>>
Expand All @@ -2697,13 +2730,10 @@ namespace ranges {

struct _Crend_fn {
#if _HAS_CXX23
template <_Should_range_access _Ty,
class _Uty = decltype(_RANGES rend(_RANGES _Possibly_const_range(_STD declval<_Ty&>())))>
template <_Range_accessible_and_rend_adaptable _Ty>
_NODISCARD constexpr auto operator()(_Ty&& _Val) const
noexcept(noexcept(const_sentinel<_Uty>{_RANGES rend(_RANGES _Possibly_const_range(_Val))}))
requires requires { const_sentinel<_Uty>{_RANGES rend(_RANGES _Possibly_const_range(_Val))}; }
{
return const_sentinel<_Uty>{_RANGES rend(_RANGES _Possibly_const_range(_Val))};
noexcept(noexcept(const_sentinel<_Rend_on_const<_Ty>>{_RANGES rend(_RANGES _Possibly_const_range(_Val))})) {
return const_sentinel<_Rend_on_const<_Ty>>{_RANGES rend(_RANGES _Possibly_const_range(_Val))};
}
#else // ^^^ C++23 / C++20 vvv
template <class _Ty, class _CTy = _Const_thru_ref<_Ty>>
Expand Down Expand Up @@ -2922,13 +2952,19 @@ namespace ranges {
_EXPORT_STD inline constexpr _Data::_Cpo data;
}

#if _HAS_CXX23
// TRANSITION, LLVM-55945
template <class _Ty>
concept _Range_accessible_and_data_adaptable =
_Should_range_access<_Ty>
&& requires(_Ty& _Val) { _RANGES _As_const_pointer(_RANGES data(_RANGES _Possibly_const_range(_Val))); };
#endif // _HAS_CXX23

struct _Cdata_fn {
#if _HAS_CXX23
template <_Should_range_access _Ty>
template <_Range_accessible_and_data_adaptable _Ty>
_NODISCARD constexpr auto operator()(_Ty&& _Val) const
noexcept(noexcept(_RANGES data(_RANGES _Possibly_const_range(_Val))))
requires requires { _RANGES _As_const_pointer(_RANGES data(_RANGES _Possibly_const_range(_Val))); }
{
noexcept(noexcept(_RANGES data(_RANGES _Possibly_const_range(_Val)))) {
return _RANGES _As_const_pointer(_RANGES data(_RANGES _Possibly_const_range(_Val)));
Comment on lines +2967 to 2968
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change requested: I observe that the noexcept expression differs from the return expression because _As_const_pointer is unconditionally noexcept - this is a reasonable shortcut for throughput.

}
#else // ^^^ C++23 / C++20 vvv
Expand Down
14 changes: 14 additions & 0 deletions tests/std/tests/P0896R4_ranges_range_machinery/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,20 @@ struct initially_incomplete;
extern initially_incomplete array_of_incomplete[42];
STATIC_ASSERT(ranges::size(array_of_incomplete) == 42);
STATIC_ASSERT(!ranges::empty(array_of_incomplete));

// begin, end, rbegin, rend, and data (and their c variations) should reject rvalues of array of incomplete elements
// with substitution failures
STATIC_ASSERT(!CanBegin<initially_incomplete (&&)[42]>);
STATIC_ASSERT(!CanCBegin<initially_incomplete (&&)[42]>);
STATIC_ASSERT(!CanEnd<initially_incomplete (&&)[42]>);
STATIC_ASSERT(!CanCEnd<initially_incomplete (&&)[42]>);
STATIC_ASSERT(!CanRBegin<initially_incomplete (&&)[42]>);
STATIC_ASSERT(!CanCRBegin<initially_incomplete (&&)[42]>);
STATIC_ASSERT(!CanREnd<initially_incomplete (&&)[42]>);
STATIC_ASSERT(!CanCREnd<initially_incomplete (&&)[42]>);
STATIC_ASSERT(!CanData<initially_incomplete (&&)[42]>);
STATIC_ASSERT(!CanCData<initially_incomplete (&&)[42]>);

struct initially_incomplete {};
initially_incomplete array_of_incomplete[42];

Expand Down