[libcxx-commits] [PATCH] D141222: [libc++] Use __verbose_abort instead of std::abort in __throw_ functions

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a subscriber: libc++ vendors.
ldionne added a comment.

This is great. I actually thought I had a local patch doing this, but I can't find it. At least it's been on my mind for a while, and in fact it was the main reason why I generalized from `__libcpp_assertion_handler` to `__libcpp_verbose_abort`.

One thing to keep in mind is that calling `__libcpp_verbose_abort` may generate larger code than calling `abort()` did, which could be annoying for folks compiling with `-fno-exceptions`. I think we may want to e.g. mark `__libcpp_verbose_abort` as `cold` to help the compiler optimize it better when possible, but at the end of the day, the calling convention for variadic functions on some architectures like arm64 isn't super size-friendly. I don't think this should prevent us from making this change, but size regressions should be on our radar.

Pinging vendors just for awareness, in particular I think Google doesn't use exceptions so they may be able to give us some early feedback on this change.



================
Comment at: libcxx/include/future:532
 #else
-    ((void)__ev);
-    _VSTD::abort();
+    std::__libcpp_verbose_abort(
+        "future_error was thrown in -fno-excpetions mode with error code %s", make_error_code(__ev));
----------------
You're using the `%s` format specifier but you're passing a `std::error_code`, which seems wrong.


================
Comment at: libcxx/include/future:533
+    std::__libcpp_verbose_abort(
+        "future_error was thrown in -fno-excpetions mode with error code %s", make_error_code(__ev));
 #endif
----------------



================
Comment at: libcxx/include/ios:455
 #else
-    ((void)__msg);
-    _VSTD::abort();
+    std::__libcpp_verbose_abort("ios_base::failue was thrown in -fno-exceptions mode with message \"%s\"", __msg);
 #endif
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141222