Also adds std::iter_common_reference_t and std::indirect_result_t.
Implements parts of:
- P0896R4 The One Ranges Proposal
Details
Details
- Reviewers
zoecarver EricWF Mordante cjdb - Group Reviewers
Restricted Project - Commits
- rG58b29a4efc22: [libc++] Add all indirect callable concepts and projected
Diff Detail
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Unit Tests
Event Timeline
| libcxx/include/__iterator/indirect_callable.h | ||
|---|---|---|
| 31 ↗ | (On Diff #340434) | Please _Uglify and the rest of this file. |
| libcxx/test/std/iterators/iterator.requirements/indirectcallable/indirectcallable.indirectinvocable/indirect_result_t.compile.pass.cpp | ||
| 26 ↗ | (On Diff #340434) | I would like to see some tests returning cv-qualified references. |
| libcxx/test/std/iterators/iterator.requirements/indirectcallable/indirectcallable.indirectinvocable/indirect_result_t.compile.pass.cpp | ||
|---|---|---|
| 26 ↗ | (On Diff #340434) |
There are some other interesting types in is_invocable.pass.cpp. Maybe one where we pick the correct overload, where there's a variadic (or even a template) involved, an explicit operator, a deleted overload, etc. Would it make any sense to have a test where the result is void? Also, maybe a few more negatives. Right now the only way we check that Is is indirectly_readable is that it's not int. That could be implemented typename std::iter_reference_t only. How about a type that is dereferenceable, but not iter_moveable. |
| libcxx/test/std/iterators/iterator.requirements/indirectcallable/indirectinvocable/indirect_strict_weak_order.compile.pass.cpp | ||
|---|---|---|
| 1 | This is basically copy-pasted from equivalence_relation because both are the same modulo semantic requirements. | |
| libcxx/test/std/iterators/iterator.requirements/indirectcallable/indirectinvocable/indirectly_regular_unary_invocable.compile.pass.cpp | ||
| 1 | This is basically copy-pasted from indirect_unary_invocable because both are the same modulo semantic requirements. | |
Comment Actions
LGTM. Thanks.
Only suggestion, does it make sense to add one or two tests where we are using this as a user would, instead of just checking the requirements are there? Maybe something like:
template<std::input_iterator I, std::sentinel_for<I> S,
std::indirectly_unary_invocable<I> Fun>
constexpr auto myForEach(I first, S last, Fun f) const {
for (; first != last; ++first) {
std::invoke(f, *first);
}
return {first, f};
}Or you could even test std::projected by projecting f.
I suppose these will come when we implement algs, such as for_each, so maybe it doesn't make sense.
| libcxx/test/std/iterators/iterator.requirements/indirectcallable/indirectinvocable/indirectly_regular_unary_invocable.compile.pass.cpp | ||
|---|---|---|
| 83 | I won't object, but I feel that the volatile tests are a bit superfluous. I'm not sure they add much. | |
Comment Actions
Yeah, I think it makes sense. I did it somewhat differently though, by adding asserts for a few less artificial predicates like [](int i) { return i % 2 == 0; } and simple things like that.
| libcxx/test/std/iterators/iterator.requirements/indirectcallable/indirectinvocable/indirectly_regular_unary_invocable.compile.pass.cpp | ||
|---|---|---|
| 83 | Agreed - they were there in the original patch I commandeered, but I am OK removing them. | |
I've got a big change coming for this csv, so I'd prefer to add it in mine (it'll screw up my doc right now).