[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#3015535 <https://reviews.llvm.org/D103947#3015535>, @ldionne wrote:

> 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?

That sounds sensible to me. It's a bit of time since I dove into it so I don't directly recall all the hairly details, but it sounds like it could work yeah.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103947