[libcxx-commits] [PATCH] D96477: [libcxx] adds remaining callable concepts

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver accepted this revision as: zoecarver.
zoecarver added a comment.

> I plan to go back at some point and add subsumption tests for all concepts. I think there are quite a few that would benefit from being in their own source files for brevity's sake, and wanna keep that consistent.
> Consider this a pilot?

As discussed offline, I'm OK a) with these tests and b) with doing them in another file.

Just for the sake of posterity: in general, I don't like tests that say "here is the implementation re-written exactly the same way, assert these two implementations look the same." That's what I thought this was at first (and there's an argument to be made that it is still that to a certain degree), but I think it's OK here because the constraints need to be entirely identical. Essentially what we're testing is that `std::relation` (for example) is implemented using `std::predicate` and not some helper concept that looks //exactly// like `std::predicate`.

I'll let @Quuxplusone give the final approval.



================
Comment at: libcxx/include/concepts:402
+// [concept.predicate]
+template<class _Fn, class... _Args>
+concept predicate =
----------------
cjdb wrote:
> zoecarver wrote:
> > Do we need to update the synopsis with these?
> I added these in the very first `<concepts>` commit (which is different to how I'm doing things for `<iterator>` and `<ranges>`). Unless there's some divergence between synopsis and code that you're pointing out that I'm missing?
Ah, missed that. This looks good then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96477