[libcxx-commits] [PATCH] D132312: [libc++][spaceship] Implement `operator<=>` for `list`

[Adrian Vogelsgesang via Phabricator via libcxx-commits]

avogelsgesang added inline comments.


================
Comment at: libcxx/test/std/containers/sequences/list/compare.pass.cpp:47
+  AssertComparisonsReturnBool<std::vector<ElemT> >();
+#if TEST_STD_VER >= 20
+  AssertOrderReturn<Order, std::vector<ElemT>>();
----------------
Mordante wrote:
> mumbleskates wrote:
> > I recommend simply adding another file, such as `compare.three_way.pass.cpp`, that marks standard versions before 20 as unsupported, then simply does the test. Adding the parts of the test you want to conditionally execute with dozens of conditional directives is honestly too much noise with no real benefit.
> > 
> > Don't forget the SFINAE tests! They should be pretty easy to do here as well. We should be able to test ordering behavior and SFINAE for all of the following:
> > 
> > 1. strongly ordered types
> > 2. partially ordered types
> > 3. weakly ordered types
> > 4. synth-ordered types (weak_ordering)
> > 5. custom user-ordering types (also causes synthesized weak_ordering)
> > 6. unordered types (SFINAE should cause the operator to not participate)
> > 
> > All of these should be trivially reusable or easily copyable to the other container types.
> I like the general idea! How about instead add a generic header file and make it possible to use the functions in there for the tests? Like we do in the `test_comparisons.h` header. That way the tests are at the expected location.
> 
> WDYT?
Good idea. I introduced `test_container_comparisons.h` to contain those test helpers. In the future, we should also add a similar utility to test containers like `std::map`, `std::set`.

The test functions are already written with `std::vector` in mind, i.e. they are `constexpr` and return `true`, so they can be used to test the constexpr-ness of `std::vector`.

> custom user-ordering types (also causes synthesized weak_ordering)
How do I write such a user-ordering?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132312