[libcxx] modifies `_CmpUnspecifiedParam` ignore types outside its domain

[cjdb]

Summary

D85051's honeypot solution was a bit too aggressive swallowed up the
comparison types, which made comparing objects of different ordering
types ambiguous.

Depends on D101707.

[tcanens]

rejected both nullptr_t ([cmp.categories]/p3 notes that "nullptr_t meets this requirement" at the end of the paragraph)

So? That note is saying that it is conforming for an implementation to use nullptr_t as _CmpUnspecifiedParam, not that users are allowed to compare against nullptr_t.

[cjdb]

removes the erroneous expansion to accommodate nullptr_t.

[cjdb]

rebases to activate CI

[cjdb]

rebases to activate CI

[cjdb]

Ping

[Mordante]

libcxx/test/std/language.support/cmp/cmp.categories.pre/zero_type.verify.cpp

35

Can you split the test macro in 2 marcos, a PASS and a FAIL? Then it's clear which of the tests are expected to a be valid and which should fail.

[cjdb]

Turns TEST_OP into TEST_FAIL and adds TEST_PASS (requested by @Mordante)

[• Quuxplusone]

libcxx/test/std/language.support/cmp/cmp.categories.pre/zero_type.verify.cpp

24

Minor clang-format-induced misformatting here.

libcxx/test/std/language.support/cmp/cmp.strongord/strongord.pass.cpp

56–57

Btw, it is bizarre to me that (throughout the existing code) we take auto& of all these values, instead of making copies with auto. Is there a subtle reason for the & to be here (throughout)? My only wild guess is that it's something broken about constexpr?

[Mordante]

libcxx/test/std/language.support/cmp/cmp.categories.pre/zero_type.verify.cpp

21

I think I wasn't clear what I meant for the TEST_PASS. After your change the 0L is valid. This is the reason the number of errors reduced from 18 to 12. So after removing the two lines above the test still passes. This was the part I wanted to have in TEST_PASS.

Is allowing 0L really intended? Looking at D85051 it seems @rsmith's intention to prohibit 0L.

If it's intended please adjust the unit test, the patch description, and the unit tests to split the TEST_FAIL unit test in a fail and pass part..

60

Even if I didn't ask for this macro, I still like it better than the previous code :-)

[cjdb]

ensures 0L fails

[cjdb]

libcxx/test/std/language.support/cmp/cmp.categories.pre/zero_type.verify.cpp

21

Fixed, although I'm not too happy with the solution. Feels kinda hacky?

[cjdb]

applies @Mordante's offline feedback to fix hack

[Mordante]

LGTM, please make sure the build passes before committing.

[cjdb]

tries to get CI on AppleClang working one more

libcxx/include/compare

 enum class _LIBCPP_ENUM_VIS _NCmpResult : signed char {
   __unordered = -127
 };
 
 struct _CmpUnspecifiedParam {
   _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEVAL
   _CmpUnspecifiedParam(int _CmpUnspecifiedParam::*) noexcept {}
 
-  template<typename _Tp, typename = _VSTD::enable_if_t<!_VSTD::is_same_v<_Tp, int>>>
+  template<class _Tp, class = enable_if_t<(!is_same_v<_Tp, int> && is_convertible_v<_Tp, nullptr_t>)>>
   _CmpUnspecifiedParam(_Tp) = delete;
 };
 
 class partial_ordering {
   using _ValueT = signed char;
 
   _LIBCPP_INLINE_VISIBILITY
   explicit constexpr partial_ordering(_EqResult __v) noexcept

libcxx/test/std/language.support/cmp/cmp.categories.pre/zero_type.verify.cpp

 
 // Ensure we reject all cases where an argument other than a literal 0 is used
 // for a comparison against a comparison category type.
 
 #include <compare>
 
 #define TEST_OP(v, op)                                                         \
   void(v op 0L);                                                               \
   void(0L op v);                                                               \
   void(v op nullptr);                                                          \
   void(nullptr op v);                                                          \
   void(v op(1 - 1));                                                           \
   void((1 - 1) op v);
 
 template <typename T>
 void test_category(T v) {
-  TEST_OP(v, ==);  // expected-error 18 {{}}
-  TEST_OP(v, !=);  // expected-error 18 {{}}
-  TEST_OP(v, <);   // expected-error 18 {{}}
-  TEST_OP(v, <=);  // expected-error 18 {{}}
-  TEST_OP(v, >);   // expected-error 18 {{}}
-  TEST_OP(v, >=);  // expected-error 18 {{}}
-  TEST_OP(v, <=>); // expected-error 18 {{}}
+  TEST_OP(v, ==);  // expected-error 12 {{}}
+  TEST_OP(v, !=);  // expected-error 12 {{}}
+  TEST_OP(v, <);   // expected-error 12 {{}}
+  TEST_OP(v, <=);  // expected-error 12 {{}}
+  TEST_OP(v, >);   // expected-error 12 {{}}
+  TEST_OP(v, >=);  // expected-error 12 {{}}
+  TEST_OP(v, <=>); // expected-error 12 {{}}
 
   void(v == 0);
   void(0 == v);
   void(v != 0);
   void(0 != v);
   void(v < 0);
   void(0 < v);
   void(v <= 0);
   void(0 <= v);
   void(v > 0);
   void(0 > v);
   void(v >= 0);
   void(0 >= v);
 #ifndef _LIBCPP_HAS_NO_SPACESHIP_OPERATOR
   void(v <=> 0);
   void(0 <=> v);
 #endif
 }
 
 int main(int, char**) {
   test_category(std::strong_ordering::equivalent);
   test_category(std::weak_ordering::equivalent);
   test_category(std::partial_ordering::equivalent);
   return 0;
 }

libcxx/test/std/language.support/cmp/cmp.partialord/partialord.pass.cpp

 #ifndef TEST_HAS_NO_SPACESHIP_OPERATOR
   ASSERT_NOEXCEPT(0 <=> Eq);
   ASSERT_NOEXCEPT(Eq <=> 0);
   ASSERT_SAME_TYPE(decltype(Eq <=> 0), std::partial_ordering);
   ASSERT_SAME_TYPE(decltype(0 <=> Eq), std::partial_ordering);
 #endif
 }
 
+constexpr void test_equality() {
+#ifndef TEST_HAS_NO_SPACESHIP_OPERATOR
+  auto& PartialEq = std::partial_ordering::equivalent;
+  auto& WeakEq = std::weak_ordering::equivalent;
+  assert(PartialEq == WeakEq);
+
+  auto& StrongEq = std::strong_ordering::equal;
+  assert(PartialEq == StrongEq);
+#endif
+}
+
 constexpr bool test_constexpr() {
   auto& Eq = std::partial_ordering::equivalent;
   auto& Less = std::partial_ordering::less;
   auto& Greater = std::partial_ordering::greater;
   auto& Unord = std::partial_ordering::unordered;
   struct {
     std::partial_ordering Value;
     bool ExpectEq;
static_assert(std::partial_ordering::unordered !=
                   std::partial_ordering::less);
     static_assert(std::partial_ordering::unordered !=
                   std::partial_ordering::equivalent);
     static_assert(std::partial_ordering::unordered !=
                   std::partial_ordering::greater);
     static_assert(std::partial_ordering::unordered ==
                   std::partial_ordering::unordered);
   }
+
+  test_equality();
 #endif
 
   return true;
 }
 
 int main(int, char**) {
   test_static_members();
   test_signatures();
+  test_equality();
   static_assert(test_constexpr(), "constexpr test failed");
 
   return 0;
 }

libcxx/test/std/language.support/cmp/cmp.strongord/strongord.pass.cpp

 #ifndef TEST_HAS_NO_SPACESHIP_OPERATOR
   ASSERT_NOEXCEPT(0 <=> Eq);
   ASSERT_NOEXCEPT(Eq <=> 0);
   ASSERT_SAME_TYPE(decltype(Eq <=> 0), std::strong_ordering);
   ASSERT_SAME_TYPE(decltype(0 <=> Eq), std::strong_ordering);
 #endif
 }
 
+constexpr void test_equality() {
+#ifndef TEST_HAS_NO_SPACESHIP_OPERATOR
+  auto& StrongEq = std::strong_ordering::equal;
+  auto& PartialEq = std::partial_ordering::equivalent;
+  assert(StrongEq == PartialEq);
+
+  auto& WeakEq = std::weak_ordering::equivalent;
+  assert(StrongEq == WeakEq);
+#endif
+}
+
 constexpr bool test_conversion() {
   static_assert(std::is_convertible<const std::strong_ordering&,
       std::partial_ordering>::value, "");
   { // value == 0
     auto V = std::strong_ordering::equivalent;
     std::partial_ordering WV = V;
     assert(WV == 0);
   }
for (auto TC : SpaceshipTestCases)
     static_assert(std::strong_ordering::equal == std::strong_ordering::equal);
     static_assert(std::strong_ordering::equal != std::strong_ordering::greater);
 
     static_assert(std::strong_ordering::greater != std::strong_ordering::less);
     static_assert(std::strong_ordering::greater != std::strong_ordering::equal);
     static_assert(std::strong_ordering::greater ==
                   std::strong_ordering::greater);
   }
+
+  test_equality();
 #endif
 
   return true;
 }
 
 int main(int, char**) {
   test_static_members();
   test_signatures();
+  test_equality();
   static_assert(test_conversion(), "conversion test failed");
   static_assert(test_constexpr(), "constexpr test failed");
 
   return 0;
 }

libcxx/test/std/language.support/cmp/cmp.weakord/weakord.pass.cpp

constexpr bool test_conversion() {
   { // value > 0
     auto V = std::weak_ordering::greater;
     std::partial_ordering WV = V;
     assert(WV > 0);
   }
   return true;
 }
 
+constexpr void test_equality() {
+#ifndef TEST_HAS_NO_SPACESHIP_OPERATOR
+  auto& WeakEq = std::weak_ordering::equivalent;
+  auto& PartialEq = std::partial_ordering::equivalent;
+  assert(WeakEq == PartialEq);
+
+  auto& StrongEq = std::strong_ordering::equal;
+  assert(WeakEq == StrongEq);
+#endif
+}
+
 constexpr bool test_constexpr() {
   auto& Eq = std::weak_ordering::equivalent;
   auto& Less = std::weak_ordering::less;
   auto& Greater = std::weak_ordering::greater;
   struct {
     std::weak_ordering Value;
     bool ExpectEq;
     bool ExpectNeq;
for (auto TC : SpaceshipTestCases)
     static_assert(std::weak_ordering::equivalent !=
                   std::weak_ordering::greater);
 
     static_assert(std::weak_ordering::greater != std::weak_ordering::less);
     static_assert(std::weak_ordering::greater !=
                   std::weak_ordering::equivalent);
     static_assert(std::weak_ordering::greater == std::weak_ordering::greater);
   }
+
+  test_equality();
 #endif
 
   return true;
 }
 
 int main(int, char**) {
   test_static_members();
   test_signatures();
+  test_equality();
   static_assert(test_conversion(), "conversion test failed");
   static_assert(test_constexpr(), "constexpr test failed");
 
   return 0;
 }