[libcxx][test] optional's comparisons with optional are not portably constrained

[CaseyCarter]

Summary

so use them as concept test cases only with libc++.

[• Quuxplusone]

libcxx/test/std/concepts/concepts.compare/concept.equalitycomparable/equality_comparable.compile.pass.cpp

34

After my suggestion below, I think this can go away again.

91–110

The test for map shouldn't be guarded by _LIBCPP_HAS_NO_THREADS. The tests involving lock_guard and mutex are just plain silly. The test for optional<int> should be portable even to MSVC (right?).

libcxx/test/std/concepts/concepts.compare/concepts.totallyordered/totally_ordered.pass.cpp

115–126

I think "commented-out static_assert" is a much better idiom for "hey we need to fix this" purposes, than "static_assert confusingly guarded under _LIBCPP_VERSION."

But I see what you mean about std::totally_ordered<std::optional<A>> — that's true for libc++ but not necessarily for everyone. What about e.g. std::list<A> — is that assertion true for everyone (according to the paper standard)? https://eel.is/c++draft/deque.syn implies to me that it is true for everyone, because operator<=>'s return type is something that SFINAEs away when T isn't totally ordered with itself, right?

I think the appropriate diff here is simply to remove old line 120.

(Alternatively, we could change old line 120 from static_assert to LIBCPP_STATIC_ASSERT, but that feels like sneaking a libc++-specific test into test/std/. If we actually care about the SFINAE-friendliness of optional<T>'s relops (in C++17 as well as C++20), then we should write a test/libcxx/ test for that SFINAE-friendliness. IMO it does not belong shoehorned into a random concepts.compare/ test.)

[• Quuxplusone]

libcxx/test/std/concepts/concepts.compare/concept.equalitycomparable/equality_comparable.compile.pass.cpp

29–32

This can go away.

[CaseyCarter]

Address review comments.

libcxx/test/std/concepts/concepts.compare/concepts.totallyordered/totally_ordered.pass.cpp

115–126

I think "commented-out static_assert" is a much better idiom for "hey we need to fix this" purposes, than "static_assert confusingly guarded under _LIBCPP_VERSION."

The instructions said "Uncomment when operator<=> is implemented for each of these types", so I did =)

But I see what you mean about std::totally_ordered<std::optional<A>> — that's true for libc++ but not necessarily for everyone. What about e.g. std::list<A> — is that assertion true for everyone (according to the paper standard)? https://eel.is/c++draft/deque.syn implies to me that it is true for everyone, because operator<=>'s return type is something that SFINAEs away when T isn't totally ordered with itself, right?

Yes, the comparison operators for containers are all generated from constrained <=> (which is synthesized from < and == on the elements if they don't have <=>) in C++20. IIUC, libc++ hasn't yet implemented all of the <=> for Standard Library types.

I think the appropriate diff here is simply to remove old line 120.

Will do.

[• Quuxplusone]

LGTM now! And fairly confident the CI failure is unrelated and long-gone, although if you want to reupload to poke CI before landing, that would also be cool. :)

[CaseyCarter]

In D116884#3308967, @Quuxplusone wrote:

LGTM now! And fairly confident the CI failure is unrelated and long-gone, although if you want to reupload to poke CI before landing, that would also be cool. :)

Nah, I live on the edge.

libcxx/test/std/concepts/concepts.compare/concept.equalitycomparable/equality_comparable.compile.pass.cpp

 #include <map>
 #include <memory>
 #include <optional>
 #include <set>
 #include <unordered_map>
 #include <unordered_set>
 #include <vector>
 
 #ifndef _LIBCPP_HAS_NO_THREADS
 #   include <mutex>
 #endif
 
 #include "compare_types.h"
+#include "test_macros.h"
 
 namespace fundamentals {
 static_assert(std::equality_comparable<int>);
 static_assert(std::equality_comparable<double>);
 static_assert(std::equality_comparable<void*>);
 static_assert(std::equality_comparable<char*>);
 static_assert(std::equality_comparable<char const*>);
 static_assert(std::equality_comparable<char volatile*>);

 static_assert(std::equality_comparable<int (S::*)() const volatile&&>);
 static_assert(
     std::equality_comparable<int (S::*)() const volatile && noexcept>);
 
 static_assert(!std::equality_comparable<void>);
 } // namespace fundamentals
 
 namespace standard_types {
 static_assert(std::equality_comparable<std::array<int, 10> >);
 static_assert(std::equality_comparable<std::deque<int> >);
 static_assert(std::equality_comparable<std::forward_list<int> >);
 static_assert(std::equality_comparable<std::list<int> >);
 
 #ifndef _LIBCPP_HAS_NO_THREADS
 static_assert(!std::equality_comparable<std::lock_guard<std::mutex> >);
 static_assert(std::equality_comparable<std::map<int, void*> >);
 static_assert(!std::equality_comparable<std::mutex>);
-static_assert(
+LIBCPP_STATIC_ASSERT(
     !std::equality_comparable<std::optional<std::lock_guard<std::mutex> > >);
-static_assert(!std::equality_comparable<std::optional<std::mutex> >);
+LIBCPP_STATIC_ASSERT(!std::equality_comparable<std::optional<std::mutex> >);
 #endif
 
-static_assert(std::equality_comparable<std::optional<int> >);
+LIBCPP_STATIC_ASSERT(std::equality_comparable<std::optional<int> >);
 static_assert(std::equality_comparable<std::set<int> >);
 static_assert(std::equality_comparable<std::unordered_map<int, void*> >);
 static_assert(std::equality_comparable<std::unordered_set<int> >);
 static_assert(std::equality_comparable<std::vector<bool> >);
 static_assert(std::equality_comparable<std::vector<int> >);
 } // namespace standard_types
 
 namespace types_fit_for_purpose {
 static_assert(std::equality_comparable<cxx20_member_eq>);
 static_assert(std::equality_comparable<cxx20_friend_eq>);
 static_assert(std::equality_comparable<member_three_way_comparable>);
 static_assert(std::equality_comparable<friend_three_way_comparable>);
 static_assert(std::equality_comparable<explicit_operators>);

libcxx/test/std/concepts/concepts.compare/concepts.totallyordered/totally_ordered.pass.cpp

 static_assert(models_totally_ordered<std::set<int> >());
 static_assert(models_totally_ordered<std::vector<bool> >());
 static_assert(models_totally_ordered<std::vector<int> >());
 
 static_assert(!std::totally_ordered<std::unordered_map<int, void*> >);
 static_assert(!std::totally_ordered<std::unordered_set<int> >);
 
 struct A {};
-// FIXME(cjdb): uncomment when operator<=> is implemented for each of these types.
-// static_assert(!std::totally_ordered<std::array<A, 10> >);
-// static_assert(!std::totally_ordered<std::deque<A> >);
-// static_assert(!std::totally_ordered<std::forward_list<A> >);
-// static_assert(!std::totally_ordered<std::list<A> >);
-static_assert(!std::totally_ordered<std::optional<A> >);
-// static_assert(!std::totally_ordered<std::set<A> >);
-// static_assert(!std::totally_ordered<std::vector<A> >);
+// FIXME(cjdb): Enable when operator<=> is implemented for each of these types.
+#ifndef _LIBCPP_VERSION
+static_assert(!std::totally_ordered<std::array<A, 10> >);
+static_assert(!std::totally_ordered<std::deque<A> >);
+static_assert(!std::totally_ordered<std::forward_list<A> >);
+static_assert(!std::totally_ordered<std::list<A> >);
+#endif // _LIBCPP_VERSION
+LIBCPP_STATIC_ASSERT(!std::totally_ordered<std::optional<A> >);
+#ifndef _LIBCPP_VERSION
+static_assert(!std::totally_ordered<std::set<A> >);
+static_assert(!std::totally_ordered<std::vector<A> >);
+#endif // _LIBCPP_VERSION
 } // namespace standard_types
 
 namespace types_fit_for_purpose {
 static_assert(models_totally_ordered<member_three_way_comparable>());
 static_assert(models_totally_ordered<friend_three_way_comparable>());
 static_assert(models_totally_ordered<explicit_operators>());
 static_assert(models_totally_ordered<different_return_types>());
 static_assert(!std::totally_ordered<cxx20_member_eq>);