[libcxx-commits] [PATCH] D118620: [SystemZ][z/OS] Build several exception derived classes as a separate library

[Zibi Sarbino via Phabricator via libcxx-commits]

zibi added a comment.

Addressing comments...



================
Comment at: libcxx/include/__memory/shared_ptr.h:50
 
+_LIBCPP_BEGIN_NAMESPACE_EXCEPTION
+
----------------
EricWF wrote:
> I don't think we want to formalize this different exception namespace. Especially as a versioned one (which it is by default).
> 
> The correct thing to do is place these types directly in namespace `std`, so their mangling never changes. `bad_weak_ptr` should have never gone in the versioned namespace, but that mistake was made, and now we're stuck with it (for now).
> 
> I'm OK with this change, but lets be verbose and do the ugly `#ifdef` blocks and namespace switching directly around `bad_weak_ptr`.
> 
> 
sure


================
Comment at: libcxx/src/CMakeLists.txt:138
+
+if (NOT ZOS)
+  list(APPEND LIBCXX_SOURCES ${LIBCXX_EXCEPTIONS_SOURCE})
----------------
EricWF wrote:
> This should be gated on the other CMake switch you're introducing, no?
Not really, we build the library multiple times to support ASCII and EBCDIC but the exception is being build  only once in EBCDIC encoding.  Using other CMake switch will build cxx with and without exception. On z/OS we don't want to build exception as part of cxx. This is the fundamental reason why we doing this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118620