@llvm/pr-subscribers-libcxx

Author: None (itrofimow)

diff --git a/libcxx/include/__exception/exception_ptr.h b/libcxx/include/__exception/exception_ptr.h
index dac5b00b57fe3..167aa4d9b367a 100644
--- a/libcxx/include/__exception/exception_ptr.h
+++ b/libcxx/include/__exception/exception_ptr.h
@@ -92,7 +92,8 @@ class _LIBCPP_EXPORTED_FROM_ABI exception_ptr {
 template <class _Ep>
 _LIBCPP_HIDE_FROM_ABI exception_ptr make_exception_ptr(_Ep __e) _NOEXCEPT {
 #  if _LIBCPP_HAS_EXCEPTIONS
-#    if _LIBCPP_AVAILABILITY_HAS_INIT_PRIMARY_EXCEPTION && __cplusplus >= 201103L
+  // Clang treats throwing ObjC types differently, and we have to preserve original throw-ing behavior
+#    if !defined(__OBJC__) && _LIBCPP_AVAILABILITY_HAS_INIT_PRIMARY_EXCEPTION && __cplusplus >= 201103L
   using _Ep2 = __decay_t<_Ep>;
 
   void* __ex = __cxxabiv1::__cxa_allocate_exception(sizeof(_Ep));

It's not great to condition only on __OBJC__ preprocessor define, since that'll create a potential ODR issue for std::make_exception_ptr on a C++ type, when you're mixing a TU built with C++ and another TU built with ObjC++ in the same program (which, everyone is).

If you switch to the throw impl only for ObjC object-pointers, that stops being a problem, as an ObjC object pointer type cannot exist in a non-ObjC++ TU.

As a refinement to my suggestion on the issue, I think std::is_pointer_v<_Ep> && std::is_convertible_v<_Ep, id> would be better (so as to not accidentally trigger on a C++ value with a user-defined conversion operator id()).

Are you still planning to update this patch?

Hi! Yes, sorry, just got back to office

I was thinking, wouldn't just std::is_pointer_v<_Ep> condition be better it terms of readability?
After all, this is just an optimization, and given that people very seldom create exception pointers from pointers, it shouldn't be that much of a performance hit, and the less odd conditions we have here the better

You mean without the OBJC ifdef? I think that would be OK. But IMO it's not actually an improvement, because the condition is no longer clearly connected with the actual reason for the existence of the alternate codepath.

I don't think CI failure has to do with these changes, @jyknight could you please have a look?