Skip to content

chunk_by_view's helper lambda returns bool #2890

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
Stephan T. Lavavej (StephanTLavavej) merged 3 commits into from
Aug 5, 2022
Merged

chunk_by_view's helper lambda returns bool #2890

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1918706
`chunk_by_view`'s helper lambda returns `bool`
cpplearner Jul 20, 2022
8a13123
Address review comments from miscco
cpplearner Jul 20, 2022
2d288cf
Casey's review comments
CaseyCarter Aug 4, 2022
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
30 changes: 22 additions & 8 deletions stl/inc/ranges
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6171,6 +6171,26 @@ namespace ranges {
inline constexpr _Slide_fn slide;
} // namespace views

template <class _Pr>
struct _Negated_pred {
template <class _Ty1, class _Ty2>
_NODISCARD constexpr bool operator()(_Ty1&& _Left, _Ty2&& _Right) {
Comment thread
CaseyCarter marked this conversation as resolved.
Comment thread
CaseyCarter marked this conversation as resolved.
return !_STD invoke(_Pred, _STD forward<_Ty1>(_Left), _STD forward<_Ty2>(_Right));
}

_Pr& _Pred;
};

template <class _Pr>
struct _Backward_negated_pred {
template <class _Ty1, class _Ty2>
_NODISCARD constexpr bool operator()(_Ty1&& _Left, _Ty2&& _Right) {
Comment thread
CaseyCarter marked this conversation as resolved.
return !_STD invoke(_Pred, _STD forward<_Ty2>(_Right), _STD forward<_Ty1>(_Left));
}

_Pr& _Pred;
};

template <forward_range _Vw, indirect_binary_predicate<iterator_t<_Vw>, iterator_t<_Vw>> _Pr>
requires view<_Vw> && is_object_v<_Pr>
class chunk_by_view : public _Cached_position<_Vw, chunk_by_view<_Vw, _Pr>> {
Expand Down Expand Up @@ -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]<class _Ty1, class _Ty2>(_Ty1&& _Left, _Ty2&& _Right) {
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});
Comment thread
CaseyCarter marked this conversation as resolved.
return _RANGES next(_Before_next, 1, _RANGES end(_Range));
}

Expand All @@ -6269,10 +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]<class _Ty1, class _Ty2>(_Ty1&& _Left, _Ty2&& _Right) {
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));
}

Expand Down
18 changes: 1 addition & 17 deletions stl/inc/xutility
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -640,30 +640,14 @@ template <class _Fn, class... _Its>
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 <indirectly_readable _It, indirectly_regular_unary_invocable<_It> _Proj>
struct projected {
using value_type = remove_cvref_t<indirect_result_t<_Proj&, _It>>;
#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();

@CaseyCarter Casey Carter (CaseyCarter) Aug 4, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For reviewers:

projected is a tool to simplify expressing the requirements of algorithms that use projections. The design intent is that the member functions don't have definitions - they are just signatures that exist for iter_reference_t et al. to look at. This hasn't worked well in practice in the real world, as witnessed by the warning suppressions for clang and MSVC around the definition, the additional bug report (VSO-1308657) mentioned there, and the fact that it broke yet again when I made Bool a local class in that PR. The PR uses projected<Bool*, identity>, and clang-cl decided to report an error about operator* being used-but-not-defined when it can't possibly be defined in another TU since Bool has internal linkage. Rather than trying to find yet another workaround for this perennial issue (four or five changes here in ~3 years?) I decided to give up and simply always define operator* to call abort. The function should never be called in practice, so even if it does end up emitted into some object files the linker should be able to clean it up.

}
#endif // ^^^ workaround ^^^
};

#ifdef __clang__
#pragma clang diagnostic pop
#endif // __clang__
#pragma warning(pop)

template <weakly_incrementable _It, class _Proj>
struct incrementable_traits<projected<_It, _Proj>> {
using difference_type = iter_difference_t<_It>;
Expand Down
18 changes: 18 additions & 0 deletions tests/std/tests/P2443R1_views_chunk_by/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,24 @@ constexpr void instantiation_test() {
#endif // TEST_EVERYTHING
}

void test_gh_2889() { // COMPILE-ONLY
// GH-2889 <ranges>: 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();
}

template <class Category, test::Common IsCommon, bool is_random = derived_from<Category, random_access_iterator_tag>>
using move_only_view = test::range<Category, const char, test::Sized{is_random}, test::CanDifference{is_random},
IsCommon, test::CanCompare{derived_from<Category, forward_iterator_tag>},
Expand Down