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

[Kent Ross via Phabricator via libcxx-commits]

mumbleskates 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
----------------
philnik wrote:
> 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.
> ```
thanks! done


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131372