[libc++][test] Mark `test_comparisons.h` helpers as nodiscard

[avogelsgesang]

Summary

I accidentally wrote testComparisons(...) instead of
assert(testComparisons(...). This compiled without issues, but
did not provide the intended test coverage. By adding a nodiscard,
we can make sure that the compiler catches such mistakes for us.

[mumbleskates]

Thanks! looks good to me

[jloser]

LGTM!

[philnik]

You shouldn't use libc++-specific macros in the tests. Instead, add a new macro to test_macros.h and use it here. Otherwise the tests that use test_comparisons.h would break for other implementations.

[avogelsgesang]

use TEST_NODISCARD instead of _LIBCPP_NODISCARD

[philnik]

LGTM with green CI. You missed a ) for the assert in the description.

libcxx/test/support/test_comparisons.h

 
 #include <type_traits>
 #include <cassert>
 #include <concepts>
 #include "test_macros.h"
 
 //  Test all six comparison operations for sanity
 template <class T, class U = T>
-TEST_CONSTEXPR_CXX14 bool testComparisons(const T& t1, const U& t2, bool isEqual, bool isLess)
+TEST_NODISCARD TEST_CONSTEXPR_CXX14 bool testComparisons(const T& t1, const U& t2, bool isEqual, bool isLess)
 {
     assert(!(isEqual && isLess) && "isEqual and isLess cannot be both true");
     if (isEqual)
         {
         if (!(t1 == t2)) return false;
         if (!(t2 == t1)) return false;
         if ( (t1 != t2)) return false;
         if ( (t2 != t1)) return false;
else /* greater */
         if ( (t2 >= t1)) return false;
         }
 
     return true;
 }
 
 //  Easy call when you can init from something already comparable.
 template <class T, class Param>
-TEST_CONSTEXPR_CXX14 bool testComparisonsValues(Param val1, Param val2)
+TEST_NODISCARD TEST_CONSTEXPR_CXX14 bool testComparisonsValues(Param val1, Param val2)
 {
     const bool isEqual = val1 == val2;
     const bool isLess  = val1  < val2;
 
     return testComparisons(T(val1), T(val2), isEqual, isLess);
 }
 
 template <class T, class U = T>

 
 template <class Order, class T, class U = T>
 constexpr void AssertOrderReturn() {
     AssertComparisonsReturnBool<T, U>();
     ASSERT_SAME_TYPE(decltype(std::declval<const T&>() <=> std::declval<const U&>()), Order);
 }
 
 template <class Order, class T, class U = T>
-constexpr bool testOrder(const T& t1, const U& t2, Order order) {
+TEST_NODISCARD constexpr bool testOrder(const T& t1, const U& t2, Order order) {
     bool equal = order == Order::equivalent;
     bool less = order == Order::less;
 
     return (t1 <=> t2 == order) && testComparisons(t1, t2, equal, less);
 }
 
 template <class T, class Param>
-constexpr bool testOrderValues(Param val1, Param val2) {
+TEST_NODISCARD constexpr bool testOrderValues(Param val1, Param val2) {
   return testOrder(T(val1), T(val2), val1 <=> val2);
 }
 
 #endif
 
 //  Test all two comparison operations for sanity
 template <class T, class U = T>
-TEST_CONSTEXPR_CXX14 bool testEquality(const T& t1, const U& t2, bool isEqual)
+TEST_NODISCARD TEST_CONSTEXPR_CXX14 bool testEquality(const T& t1, const U& t2, bool isEqual)
 {
     if (isEqual)
         {
         if (!(t1 == t2)) return false;
         if (!(t2 == t1)) return false;
         if ( (t1 != t2)) return false;
         if ( (t2 != t1)) return false;
         }
     else /* not equal */
         {
         if ( (t1 == t2)) return false;
         if ( (t2 == t1)) return false;
         if (!(t1 != t2)) return false;
         if (!(t2 != t1)) return false;
         }
 
     return true;
 }
 
 //  Easy call when you can init from something already comparable.
 template <class T, class Param>
-TEST_CONSTEXPR_CXX14 bool testEqualityValues(Param val1, Param val2)
+TEST_NODISCARD TEST_CONSTEXPR_CXX14 bool testEqualityValues(Param val1, Param val2)
 {
     const bool isEqual = val1 == val2;
 
     return testEquality(T(val1), T(val2), isEqual);
 }
 
 template <class T, class U = T>
 void AssertEqualityAreNoexcept()

libcxx/test/support/test_macros.h

 #else
 #define LIBCPP_ASSERT(...) static_assert(true, "")
 #define LIBCPP_STATIC_ASSERT(...) static_assert(true, "")
 #define LIBCPP_ASSERT_NOEXCEPT(...) static_assert(true, "")
 #define LIBCPP_ASSERT_NOT_NOEXCEPT(...) static_assert(true, "")
 #define LIBCPP_ONLY(...) static_assert(true, "")
 #endif
 
+#if __has_cpp_attribute(nodiscard)
+#  define TEST_NODISCARD [[nodiscard]]
+#else
+#  define TEST_NODISCARD
+#endif
+
 #define TEST_IGNORE_NODISCARD (void)
 
 namespace test_macros_detail {
 template <class T, class U>
 struct is_same { enum { value = 0};} ;
 template <class T>
 struct is_same<T, T> { enum {value = 1}; };
 } // namespace test_macros_detail