[libcxx-commits] [PATCH] D101277: [libcxx][iterator] adds `indirectly_[regular_]unary_invocable` and `projected`

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added a comment.

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.



================
Comment at: libcxx/test/std/iterators/iterator.requirements/indirectcallable/indirectinvocable/indirectly_regular_unary_invocable.compile.pass.cpp:83
+static_assert(std::indirectly_regular_unary_invocable<int (S::*)() const, S*>);
+static_assert(std::indirectly_regular_unary_invocable<int (S::*)() volatile, S*>);
+static_assert(std::indirectly_regular_unary_invocable<int (S::*)() const volatile, S*>);
----------------
I won't object, but I feel that the `volatile` tests are a bit superfluous. I'm not sure they add much. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101277/new/

https://reviews.llvm.org/D101277