[libcxx-commits] [PATCH] D131372: [libc++][spaceship] Implement std::variant::operator<=>

[Adrian Vogelsgesang via Phabricator via libcxx-commits]

avogelsgesang added inline comments.


================
Comment at: libcxx/test/std/utilities/variant/variant.monostate.relops/relops.pass.cpp:30
   constexpr M m2{};
-  {
-    static_assert((m1 < m2) == false, "");
-    ASSERT_NOEXCEPT(m1 < m2);
-  }
-  {
-    static_assert((m1 > m2) == false, "");
-    ASSERT_NOEXCEPT(m1 > m2);
-  }
-  {
-    static_assert((m1 <= m2) == true, "");
-    ASSERT_NOEXCEPT(m1 <= m2);
-  }
-  {
-    static_assert((m1 >= m2) == true, "");
-    ASSERT_NOEXCEPT(m1 >= m2);
-  }
-  {
-    static_assert((m1 == m2) == true, "");
-    ASSERT_NOEXCEPT(m1 == m2);
-  }
-  {
-    static_assert((m1 != m2) == false, "");
-    ASSERT_NOEXCEPT(m1 != m2);
-  }
+  testComparisons(m1, m2, /*isEqual*/ true, /*isLess*/ false);
+  AssertComparisonsAreNoexcept<M>();
----------------
`testComparisons` returns a bool indicating if it was succesful or not. The way you wrote this test case, it does not provide the intended test coverage.
The correct way to write this is

```
assert(testComparisons(m1, m2, /*isEqual*/ true, /*isLess*/ false));
```

Here and below in line 34

Also see https://reviews.llvm.org/D131364 and consider giving me a review on that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131372