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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D103947#2989849 <https://reviews.llvm.org/D103947#2989849>, @phosek wrote:

> In D103947#2915640 <https://reviews.llvm.org/D103947#2915640>, @phosek wrote:
>
>> In D103947#2911434 <https://reviews.llvm.org/D103947#2911434>, @mstorsjo wrote:
>>
>>>> To avoid this, would it be possible to provide an ABI-compatible `std::exception` class, and use that unconditionally when building on top of the VC runtime?
>>>
>>> Maybe that's possible - I haven't studied if their regular `std::exception` class does something funky or not.
>>
>> Their `std::exception` has two class members (one for the name string and other for what appears to be an deallocation flag) so we would need to conditionally modify the libc++ `std::exception` layout on Windows to match theirs.
>
> @ldionne Do you have any more thoughts on this? I'd like to get this change landed to enable the use of libc++ on Windows with `_HAS_EXCEPTIONS=0` but I don't know what's the direction we want to take.

My understanding is that the core of the issue is:

> If `_HAS_EXCEPTIONS` is set to 0, the `vcruntime_exception.h` header doesn't define the ABI base class `std::exception`.

I think we should either always use our own `std::exception` class, or always use theirs (or an ABI compatible version of theirs when exceptions are disabled). To be more concrete, today we have this in `<exception>`:

  #if !defined(_LIBCPP_ABI_VCRUNTIME)
  class _LIBCPP_EXCEPTION_ABI exception
  {
      // ...
  };
  
  class _LIBCPP_EXCEPTION_ABI bad_exception
      : public exception
  {
      // ...
  };
  #endif // !_LIBCPP_ABI_VCRUNTIME

I would do something like this instead:

  // <vcruntime_exception.h> defines its own std::exception and std::bad_exception types,
  // which we use in order to be ABI-compatible with other STLs on Windows.
  #if defined(_LIBCPP_ABI_VCRUNTIME) && _HAS_EXCEPTIONS
  #   include <vcruntime_exception.h>
  
  // However, <vcruntime_exception.h> does not define std::exception and std::bad_exception 
  // under -fno-exceptions.
  //
  // Since libc++ still wants to provide the std::exception hierarchy under -fno-exceptions
  // (after all those are simply types like any other), we define an ABI-compatible version 
  // of the VCRuntime std::exception and std::bad_exception types in that mode.
  #elif defined(_LIBCPP_ABI_VCRUNTIME) && !_HAS_EXCEPTIONS
  class _LIBCPP_EXCEPTION_ABI exception
  {
      // ABI-compatible definition with vcruntime
  };
  
  class _LIBCPP_EXCEPTION_ABI bad_exception
      : public exception
  {
      // ABI-compatible definition with vcruntime
  };
  
  // On all other platforms, we define our own std::exception and std::bad_exception types
  // regardless of whether exceptions are turned on as a language feature.
  #else
  class _LIBCPP_EXCEPTION_ABI exception
  {
      // ...
  };
  
  class _LIBCPP_EXCEPTION_ABI bad_exception
      : public exception
  {
      // ...
  };
  #endif

WDYT? Does this make sense to you? @mstorsjo do you think that's sensible, too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103947