[libc++][spaceship] Implement `operator<=>` for `multiset` and `set`

[H-G-Hristov]

Summary

Implements parts of P1614R2

Implemented operator<=> for multiset and set

[H-G-Hristov]

• Fixed CI

[H-G-Hristov]

• Fixed CI

[H-G-Hristov]

• Fixed CI

[H-G-Hristov]

Run CI

[H-G-Hristov]

• Fixed CI

[H-G-Hristov]

• Fixed CI

[H-G-Hristov]

• Fixed CI

[H-G-Hristov]

• Fixed CI

[H-G-Hristov]

• Rebased

[H-G-Hristov]

I assume this passed the CI checks, so I'd like to put it for review/discussion as is.

NOTE: Unlike any other container (including std::multisite) "unorderable" values are not added to std::set. I couldn't find an explanation for that, so I hope for a feedback. I think libstdc++'s std::set behaves in the same way.

[Mordante]

Thanks for working on this. Some comments.

libcxx/include/set

1044

libcxx/test/std/containers/associative/multiset/multiset.nonmember/compare.three_way.pass.cpp

25

Does this also test containers where Compare != std::less ?

libcxx/test/support/test_container_comparisons.h

233

Does this work with a set?

[H-G-Hristov]

Rebased

[H-G-Hristov]

@Mordante Thank you for the comments. Sorry about answering your question with questions though.

libcxx/include/set

1044

@Mordante Could you please elaborate why I need to remove inline? I am asking because operator<=> for map, list, forward_list, deque are also inline as well as the remaining operators?
If I need to remove inline here, do I need to also update the other containers for consistency?

libcxx/test/std/containers/associative/multiset/multiset.nonmember/compare.three_way.pass.cpp

25

No. I didn't consider the other cases. What other cases would you like me to add?
For consistency, I will also need to update the tests for map in that case.

libcxx/test/support/test_container_comparisons.h

233

To reduce code duplication I selected the values to work for set and multiset. I did the same for map and multimap before.
Or am I missing something?

[Mordante]

libcxx/include/set

1044

You can keep it if you want, but templates are implicitly inline. Sometimes we forget to point it out in reviews.

libcxx/test/std/containers/associative/multiset/multiset.nonmember/compare.three_way.pass.cpp

25

For the generic comparison it would be good to make sure the Compare does not matter. We probably should also test that maps with different Compare or Alloc can't be compared.

[H-G-Hristov]

• Addressed comments

[H-G-Hristov]

Addressed comments

[H-G-Hristov]

@Mordante Thank you for the review.

libcxx/test/std/containers/associative/multiset/multiset.nonmember/compare.three_way.pass.cpp

25

I think I need a start To-Do list. Thank you for suggesting!

[Mordante]

LGTM modulo one nit.

libcxx/test/support/test_comparisons.h

258–259

Unlike constexpr functions constexpr variable are not inline.

262

I actually feel std::numeric_limits<int>::min() is more readable. Especially since the name kind of indicates it's not a plain std::numeric_limits<int>::min(). So I really like this part reverted and use the numeric_limits directly.

[H-G-Hristov]

• Addressed comments

[H-G-Hristov]

@Mordante Thank you for the review. I reverted the change as you recommended.

[H-G-Hristov]

libcxx/test/support/test_comparisons.h

262

Removed both variables from above.

[H-G-Hristov]

• Addressed comments

[H-G-Hristov]

Updated // expected-error tests in *.verify.cpp according the review comments in D151205

[H-G-Hristov]

@Mordante I made a slight adjustments to the tests after your approval, if you don't mind I'll wait for a few days and I'll land it.

[Mordante]

libcxx/test/std/containers/associative/set/set.nonmember/compare.three_way.verify.cpp

45–46

Mainly for your information, for short lines you can use this too.
Since our CI has some issues at the moment feel free to land it as is.