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

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/test/support/test_macros.h:238-246
+#if TEST_STD_VER >= 17
+#  define TEST_NODISCARD [[nodiscard]]
+#elif defined(TEST_COMPILER_CLANG) || defined(TEST_COMPILER_GCC)
+#  define TEST_NODISCARD __attribute__((warn_unused_result))
+#elif defined(TEST_COMPILER_MSVC) && _MSC_VER >= 1700
+#  define TEST_NODISCARD _Check_return_
+#else
----------------
I think this should just be
```
#if __has_cpp_attribute(nodiscard)
#  define TEST_NODISCARD [[nodiscard]]
#else
#  define TEST_NODISCARD
#endif
```
I don't really see a reason to use `warn_unused_result`, since both clang and GCC have `[[nodiscard]]` as an extension since C++11 and Microsoft only tests with C++14 and later anyways AFAICT. Also, our `__config` header says:
```
// We can't use GCC's [[gnu::warn_unused_result]] and
// __attribute__((warn_unused_result)), because GCC does not silence them via
// (void) cast.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131372