[libcxx-commits] [PATCH] D120010: [libcxx][test] Neither MSVC nor its STL support int128

Thu Feb 17 12:22:59 PST 2022

CaseyCarter added inline comments.

================
Comment at: libcxx/test/support/test_macros.h:369

-#if defined(_LIBCPP_HAS_NO_INT128) || defined(TEST_COMPILER_MSVC)
+#if defined(_LIBCPP_HAS_NO_INT128) || defined(TEST_COMPILER_MSVC) || defined(_MSVC_STL_VERSION)
 #   define TEST_HAS_NO_INT128
----------------
Quuxplusone wrote:
> Do we support any compilers that predefine both `__SIZEOF_INT128__` and `_MSC_VER`, and yet //don't// support `__int128`?  I bet the answer is "no." If the answer is "no," then you should remove `|| defined(TEST_COMPILER_MSVC)` from this line, leaving only
> ```
> #if defined(_LIBCPP_HAS_NO_INT128) || defined(_MSVC_STL_VERSION)
> ```
> which sounds about right to me: it says "If this is libc++ with no int128, //or// if this is MSVC STL which never has int128, then..."  What //C++ compiler// we're using shouldn't matter at all, because we should just trust the //library// (libc++ or MSVC STL) to relay accurately the situation re int128 support.
> 
> I notice that we're implicitly assuming that libstdc++ always supports int128, which I'm sure is technically wrong; but we can leave that to the libstdc++ people to sort out. :)
> 
> Someone +1 my logic and then I'll approve this. ;)
Yes, I agree this falls out of the definition of `_LIBCPP_HAS_NO_INT128`. Fixing.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120010