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

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Fri May 28 07:08:08 PDT 2021


ldionne marked 3 inline comments as done.
ldionne added a comment.

In D101277#2785970 <https://reviews.llvm.org/D101277#2785970>, @zoecarver wrote:

> 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:

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.



================
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*>);
----------------
zoecarver wrote:
> I won't object, but I feel that the `volatile` tests are a bit superfluous. I'm not sure they add much. 
Agreed - they were there in the original patch I commandeered, but I am OK removing them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101277



More information about the libcxx-commits mailing list