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

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added a comment.

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

> so this is down to one failure: `support/test.support/test_macros_header.no_exceptions.verify.cpp` (https://buildkite.com/llvm-project/libcxx-ci/builds/10551#b46abad1-6a39-4118-8a86-b50e1aaab001).
>
> The failing test expects an error due to exceptions being disabled, so I'm not sure we should expect that to pass in this config.
>
> `_HAS_EXCEPTIONS = 0` disables exceptions by failing to provide definitions to exception classes, and is what gets set when `fno-exceptions` is used, but I don't think that compilation will fail with the expected message if you *only* set `_HAS_EXCEPTIONS = 0`, but don't also use `fno-exceptions`.
>
> @phosek @mstorsjo @ldionne what's the course of action here? just maybe mark this as XFAIL? or disable this test if in this config?

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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103947