[libcxx-commits] [PATCH] D108389: [libc++] Bypass calling exception-throwing functions in the dylib with -fno-exceptions

[Louis Dionne via Phabricator via libcxx-commits]

ldionne marked an inline comment as done.
ldionne added a comment.

In D108389#2955148 <https://reviews.llvm.org/D108389#2955148>, @mclow.lists wrote:

> What's the goal here? In terms of behavior?

So it's a little bit embarrassing, but I came across a dubious use of libc++ where they were overriding `_LIBCPP_EXTERN_TEMPLATE` with their custom thing. It should never have worked, but starting in D103964 <https://reviews.llvm.org/D103964>, it really stopped working because we are not checking for `#ifdef _LIBCPP_EXTERN_TEMPLATE` anymore. Note that this is intended - we don't want users to override `_LIBCPP_EXTERN_TEMPLATE`, and in fact I'll even move forward with the more assertive D103960 <https://reviews.llvm.org/D103960> once I've had time to clean up the debug mode. The proper solution here would be for those users to stop overriding `_LIBCPP_EXTERN_TEMPLATE` and start linking against libc++ (which they currently don't do), however they can't do that yet for complicated reasons.

So what I'm trying to do is basically remove the undefined reference that's breaking them without making the library worse for it. Technically, this change is a slight improvement over the status quo cause it removes the reliance on weak references (`__throw_xxxx`) when exceptions are disabled, and it solves the concrete problem I have at hand.

To answer your question more directly, the goal here in terms of behavior is to ensure that a TU using `std::vector::at` (and friends) under `-fno-exceptions` doesn't rely on a weak-def symbol located in the dylib -- which it technically doesn't need cause all we do is `abort()` anyway.



================
Comment at: libcxx/include/string:1720
+#if defined(_LIBCPP_NO_EXCEPTIONS)
+        _VSTD::abort();
+#else
----------------
Quuxplusone wrote:
> `#include <cstdlib>`?
Good catch, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108389