From 191870691b00591eec51545730b06f7809108958 Mon Sep 17 00:00:00 2001 From: "S. B. Tam" Date: Wed, 20 Jul 2022 16:25:07 +0800 Subject: [PATCH 1/3] `chunk_by_view`'s helper lambda returns `bool` --- stl/inc/ranges | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/stl/inc/ranges b/stl/inc/ranges index 88488143a5d..61a4e994246 100644 --- a/stl/inc/ranges +++ b/stl/inc/ranges @@ -6255,7 +6255,7 @@ namespace ranges { _STL_VERIFY(_Pred, "cannot increment a chunk_by_view iterator whose parent view has no predicate"); #endif // _ITERATOR_DEBUG_LEVEL != 0 - const auto _Not_pred = [&_Orig_pred = *_Pred](_Ty1&& _Left, _Ty2&& _Right) { + const auto _Not_pred = [&_Orig_pred = *_Pred](_Ty1&& _Left, _Ty2&& _Right) -> bool { return !_STD invoke(_Orig_pred, _STD forward<_Ty1>(_Left), _STD forward<_Ty2>(_Right)); }; const auto _Before_next = _RANGES adjacent_find(_It, _RANGES end(_Range), _Not_pred); @@ -6269,7 +6269,8 @@ namespace ranges { #endif // _ITERATOR_DEBUG_LEVEL != 0 reverse_view _Rv{subrange{_RANGES begin(_Range), _It}}; - const auto _Rev_not_pred = [&_Orig_pred = *_Pred](_Ty1&& _Left, _Ty2&& _Right) { + const auto _Rev_not_pred = [&_Orig_pred = *_Pred]( + _Ty1&& _Left, _Ty2&& _Right) -> bool { return !_STD invoke(_Orig_pred, _STD forward<_Ty2>(_Right), _STD forward<_Ty1>(_Left)); }; const auto _After_prev = _RANGES adjacent_find(_Rv, _Rev_not_pred); From 8a131234ad9601e704b6dc074bd0c43223557098 Mon Sep 17 00:00:00 2001 From: cpplearner Date: Thu, 21 Jul 2022 02:43:38 +0800 Subject: [PATCH 2/3] Address review comments from miscco --- stl/inc/ranges | 31 +++++++++++++------ .../std/tests/P2443R1_views_chunk_by/test.cpp | 18 +++++++++++ 2 files changed, 40 insertions(+), 9 deletions(-) diff --git a/stl/inc/ranges b/stl/inc/ranges index 61a4e994246..225afba439f 100644 --- a/stl/inc/ranges +++ b/stl/inc/ranges @@ -6171,6 +6171,26 @@ namespace ranges { inline constexpr _Slide_fn slide; } // namespace views + template + struct _Negated_pred { + template + _NODISCARD constexpr bool operator()(_Ty1&& _Left, _Ty2&& _Right) { + return !_STD invoke(_Pred, _STD forward<_Ty1>(_Left), _STD forward<_Ty2>(_Right)); + } + + _Pr& _Pred; + }; + + template + struct _Backward_negated_pred { + template + _NODISCARD constexpr bool operator()(_Ty1&& _Left, _Ty2&& _Right) { + return !_STD invoke(_Pred, _STD forward<_Ty2>(_Right), _STD forward<_Ty1>(_Left)); + } + + _Pr& _Pred; + }; + template , iterator_t<_Vw>> _Pr> requires view<_Vw> && is_object_v<_Pr> class chunk_by_view : public _Cached_position<_Vw, chunk_by_view<_Vw, _Pr>> { @@ -6255,10 +6275,7 @@ namespace ranges { _STL_VERIFY(_Pred, "cannot increment a chunk_by_view iterator whose parent view has no predicate"); #endif // _ITERATOR_DEBUG_LEVEL != 0 - const auto _Not_pred = [&_Orig_pred = *_Pred](_Ty1&& _Left, _Ty2&& _Right) -> bool { - return !_STD invoke(_Orig_pred, _STD forward<_Ty1>(_Left), _STD forward<_Ty2>(_Right)); - }; - const auto _Before_next = _RANGES adjacent_find(_It, _RANGES end(_Range), _Not_pred); + const auto _Before_next = _RANGES adjacent_find(_It, _RANGES end(_Range), _Negated_pred<_Pr>{*_Pred}); return _RANGES next(_Before_next, 1, _RANGES end(_Range)); } @@ -6269,11 +6286,7 @@ namespace ranges { #endif // _ITERATOR_DEBUG_LEVEL != 0 reverse_view _Rv{subrange{_RANGES begin(_Range), _It}}; - const auto _Rev_not_pred = [&_Orig_pred = *_Pred]( - _Ty1&& _Left, _Ty2&& _Right) -> bool { - return !_STD invoke(_Orig_pred, _STD forward<_Ty2>(_Right), _STD forward<_Ty1>(_Left)); - }; - const auto _After_prev = _RANGES adjacent_find(_Rv, _Rev_not_pred); + const auto _After_prev = _RANGES adjacent_find(_Rv, _Backward_negated_pred<_Pr>{*_Pred}); return _RANGES prev(_After_prev.base(), 1, _RANGES begin(_Range)); } diff --git a/tests/std/tests/P2443R1_views_chunk_by/test.cpp b/tests/std/tests/P2443R1_views_chunk_by/test.cpp index d52d5058b7d..daa7897f2fd 100644 --- a/tests/std/tests/P2443R1_views_chunk_by/test.cpp +++ b/tests/std/tests/P2443R1_views_chunk_by/test.cpp @@ -231,6 +231,24 @@ constexpr void instantiation_test() { #endif // TEST_EVERYTHING } +struct Bool { + Bool() {} + Bool(const Bool&) = delete; + Bool& operator!() { + return *this; + } + operator bool() { + return true; + } +}; + +void test_gh_2889() { // COMPILE-ONLY + // GH-2889 : chunk_by_view's helper lambda does not specify return type + Bool x[3]; + auto r = x | views::chunk_by([](Bool& b, Bool&) -> Bool& { return b; }); + (void) r.begin(); +} + template > using move_only_view = test::range}, From 2d288cf288ab347cc08cd209af3c654d6e0f9b27 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Thu, 4 Aug 2022 12:32:05 -0700 Subject: [PATCH 3/3] Casey's review comments --- stl/inc/xutility | 18 +-------------- .../std/tests/P2443R1_views_chunk_by/test.cpp | 22 +++++++++---------- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/stl/inc/xutility b/stl/inc/xutility index c88fbe7274e..a7c0ea49d86 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -640,30 +640,14 @@ template using indirect_result_t = invoke_result_t<_Fn, iter_reference_t<_Its>...>; // clang-format on -#pragma warning(push) -#pragma warning(disable : 5046) // '%s': Symbol involving type with internal linkage not defined -#ifdef __clang__ -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wundefined-internal" // function '%s' has internal linkage but is not defined -#endif // __clang__ - template _Proj> struct projected { using value_type = remove_cvref_t>; -#if defined(__clang__) || defined(__EDG__) - indirect_result_t<_Proj&, _It> operator*() const; -#else // ^^^ no workaround / workaround vvv indirect_result_t<_Proj&, _It> operator*() const { - _CSTD abort(); // TRANSITION, VSO-1308657 + _CSTD abort(); } -#endif // ^^^ workaround ^^^ }; -#ifdef __clang__ -#pragma clang diagnostic pop -#endif // __clang__ -#pragma warning(pop) - template struct incrementable_traits> { using difference_type = iter_difference_t<_It>; diff --git a/tests/std/tests/P2443R1_views_chunk_by/test.cpp b/tests/std/tests/P2443R1_views_chunk_by/test.cpp index daa7897f2fd..db69d0a4bf7 100644 --- a/tests/std/tests/P2443R1_views_chunk_by/test.cpp +++ b/tests/std/tests/P2443R1_views_chunk_by/test.cpp @@ -231,19 +231,19 @@ constexpr void instantiation_test() { #endif // TEST_EVERYTHING } -struct Bool { - Bool() {} - Bool(const Bool&) = delete; - Bool& operator!() { - return *this; - } - operator bool() { - return true; - } -}; - void test_gh_2889() { // COMPILE-ONLY // GH-2889 : chunk_by_view's helper lambda does not specify return type + struct Bool { // NB: poor model of boolean-testable; don't use in runtime code. + Bool() = default; + Bool(const Bool&) = delete; + Bool& operator!() { + return *this; + } + operator bool() { + return true; + } + }; + Bool x[3]; auto r = x | views::chunk_by([](Bool& b, Bool&) -> Bool& { return b; }); (void) r.begin();