[libcxx-commits] [PATCH] D103947: [libcxx] Fix using the vcruntime ABI with _HAS_EXCEPTIONS=0 defined

Wed May 4 13:34:04 PDT 2022

mstorsjo added a comment.

In D103947#3492146 <https://reviews.llvm.org/D103947#3492146>, @paulkirth wrote:

> In D103947#3492009 <https://reviews.llvm.org/D103947#3492009>, @mstorsjo wrote:
>
>> I think you should extend `test_macros.h` to define `TEST_HAS_EXCEPTIONS` like this:
>>
>>   #if (!TEST_HAS_FEATURE(cxx_exceptions) && !defined(__cpp_exceptions) \
>>        && !defined(__EXCEPTIONS)) || \
>>       (defined(_HAS_EXCEPTIONS) && _HAS_EXCEPTIONS == 0) 
>>   #define TEST_HAS_NO_EXCEPTIONS
>>   #endif 
>>
>> As the `_HAS_EXCEPTIONS=0` flag practically means that we should skip exception tests, so the `no-exceptions` testsuite feature flag should be set. But on the compiler level, exceptions aren't really disabled, so this test macro doesn't pick it up, but we maybe can make the test macro look at `_HAS_EXCEPTIONS` too. Or does that break other tests, if `TEST_HAS_NO_EXCEPTIONS` suddenly is defined in all tests?
>
> We already define the macro in that way.  clang-format modified the layout slightly, but they look identical to me. I haven't noticed any tests failing because of that change.

Oh, sorry, I looked at the failure too quickly, now I see what's going on. Yeah I guess it'd make most sense to XFAIL this test, somehow. Wrapping the try/catch in an `#if !defined(_HAS_EXCEPTIONS) || _HAS_EXCEPTIONS != 0` could work too but isn't quite pretty (but I don't know of a neat way of XFAILing this test either, other than adding yet another test feature flag for this specific configuration.)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103947