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

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF added a comment.

Sorry, I'm going to have to let @ldionne weigh in on this too. I'm just not sure introducing this whole new build mode with a separate exception library is something we want to do -- so I would like input from the other maintainers.

I think the correct approach here is to define the required exception symbols in libc++abi, like we do already for most exception types.

I think we're inventing something novel here because it's easy, not because it's the best approach.



================
Comment at: libcxx/include/__memory/shared_ptr.h:50
 
+namespace std {
+#else
----------------
Let's create an ABI macro, since other users might want to have the `bad_weak_ptr` exception too, that will make this code more readable and then you can just turn it on for your platform.

Sorry for not suggesting that earlier.


================
Comment at: libcxx/src/CMakeLists.txt:138
+
+if (NOT ZOS)
+  list(APPEND LIBCXX_SOURCES ${LIBCXX_EXCEPTIONS_SOURCE})
----------------
zibi wrote:
> 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.
Right, then these things should go into libc++abi. Not a third separate library.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118620