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

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/docs/Cxx2aStatusPaperStatus.csv:79
 "`P1236R1 <https://wg21.link/P1236R1>`__","CWG","Alternative Wording for P0907R4 Signed Integers are Two's Complement","San Diego","* *",""
-"`P1248R1 <https://wg21.link/P1248R1>`__","LWG","Remove CommonReference requirement from StrictWeakOrdering (a.k.a Fixing Relations)","San Diego","* *",""
+"`P1248R1 <https://wg21.link/P1248R1>`__","LWG","Remove CommonReference requirement from StrictWeakOrdering (a.k.a Fixing Relations)","San Diego","|Complete|","14.0"
 "`P1285R0 <https://wg21.link/P1285R0>`__","LWG","Improving Completeness Requirements for Type Traits","San Diego","* *",""
----------------
Should this say "13.0"?


================
Comment at: libcxx/include/concepts:447
+concept strict_weak_order = relation<_Rp, _Tp, _Up>;
+#endif //_LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_CONCEPTS)
 
----------------
Please take the edit before landing.


================
Comment at: libcxx/test/std/concepts/concepts.callable/concept.equiv/equivalence_relation.subsumption.pass.cpp:22-24
+template<class F, class T>
+requires std::equivalence_relation<F, T, T> && true
+[[nodiscard]] constexpr bool check_subsumption() { return false; }
----------------
This template-of-two-parameters is never used. Please remove it from this patch.


================
Comment at: libcxx/test/std/concepts/concepts.callable/concept.equiv/equivalence_relation.subsumption.pass.cpp:51-53
+template<class F, class T, class U>
+requires std::equivalence_relation<F, T, U>
+[[nodiscard]] constexpr bool check_no_subsumption() { return false; }
----------------
This template `check_no_subsumption` is never used. Please remove it from this patch.


================
Comment at: libcxx/test/std/concepts/concepts.callable/concept.predicate/predicate.subsumption.pass.cpp:17
+
+[[nodiscard]] constexpr bool check_subsumption(std::regular_invocable auto) {
+  return false;
----------------
cjdb wrote:
> Quuxplusone wrote:
> > I would strongly prefer to see as much of the "cruft" removed from this test as possible, so that we could have a bunch of subsumption tests. The extreme form of that would be to use the `SUBSUMES` macro from https://quuxplusone.github.io/blog/2018/09/23/member-concepts-ii/ . But I think we should actually do more like
> > https://godbolt.org/z/51edrGrM8
> > ```
> > bool model(int, int) { return true; }
> > 
> > constexpr bool relation_subsumes_predicate(std::relation auto) requires true { return true; }
> > constexpr void relation_subsumes_predicate(std::predicate auto) { }
> > static_assert( relation_subsumes_predicate(model) );
> > ```
> > in a single `.compile.fail.cpp` file, so that we can verify easily also when a subsumption relationship does //not// exist.
> > 
> > If anyone knows a clever way to detect non-subsumption in a .compile.pass.cpp test, ping me in #meta-programming or something. ;)
> > I would strongly prefer to see as much of the "cruft" removed from this test as possible, so that we could have a bunch of subsumption tests. 
> 
> It's not exactly clear to me what "cruft" you're referring to.
> It's not exactly clear to me what "cruft" you're referring to.

For the record: `// clang-format off`, `[[nodiscard]]`, `// clang-format on`... Basically anything that doesn't appear in the three-line version I posted above. (I'd even prefer to find a way to eliminate `constexpr`, but I don't think that's feasible.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96477