[libcxx-commits] [PATCH] D155598: [libc++abi] Use std::abort() instead of std::terminate() on failure to allocate

[Nick Desaulniers via Phabricator via libcxx-commits]

nickdesaulniers added a comment.

In D155598#4510499 <https://reviews.llvm.org/D155598#4510499>, @ldionne wrote:

> I have no idea how to reliably test this.
>
> I'm also looking for feedback on whether folks think this is reasonable, or whether we should pursue this alternative explained in the commit message:
>
>   Another option would be to properly handle failure-to-allocate inside
>   ItaniumDemangle.h and to propagate something like an error code or a
>   std::expected to the caller of all functions in the demangler that
>   can allocate. Then, we could make sure that cxa_demangle returns
>   nullptr when it fails to demangle the input due to any error, as it is
>   supposed to (but today "true" out-of-memory conditions are not handled
>   properly). The demangling_terminate_handler() would then see that
>   cxa_demangle failed to do its job and would still print the
>   appropriate message, simply using the non-demangled exception type.
>   However, this is akin to a partial rewrite of the demangler code since
>   a large number of functions would now have to return a std::expected
>   to account for out-of-memory conditions.

I'm all for updating the interface. +1 to @philnik 's comment.



================
Comment at: libcxxabi/src/demangle/README.txt:37
 If you're working on the generic library, then do the work first in
-libcxxabi, then run the cp-to-llvm.sh script in src/demangle. This
+libcxxabi, then run libcxxabi/src/demangle/cp-to-llvm.sh. This
 script takes as an optional argument the path to llvm, and copies the
----------------
this LGTM if you want to commit this change to libcxxabi/src/demangle/README.txt separately of this commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155598