[libcxx-commits] [libcxx] Simplify the __assertion_handler build logic. Be friendly to IDEs. (PR #93333)

[Louis Dionne via libcxx-commits]

ldionne wrote:

> I was wrong. Moving it to the target include directory doesn't work because it introduces a layering violation.
> 
> I'll do some digging into _why_ `__assertion_handler` as a separate configurable file is needed at all. There are likely other idiomatic means to allow overriding the handler.

You're welcome to, but TBH we've been there before and we basically tried all the possible different ways of customizing this before we ended up where we are. We tried link-time overriding, but that is too heavy when you need to generate a single instruction to trap (see below). We tried compile-time overriding via a macro (by defining `_LIBCPP_VERBOSE_ABORT`), but that doesn't scale if you want to override the handler for a whole platform/company/environment/stack/whatever, since everyone now has to define that macro.

So we ended up where we are. Note that any kind of configuration-time tweakability is going to throw a wrench into the IDE, I think. It's not the fact that it's a separate header that is problematic, it's the fact that there is some configuration-time tweaking of the headers at all. So even if `__assertion_handler` were to be a `.in` file that gets configured via CMake, it still wouldn't be valid C++ until it gets configured, which is what the IDE needs.

> 
> Where are the requirements on the vendor override documented?

The current design is the result of various iterations on `__libcpp_verbose_abort`, and the reason for the handler being overridable at configuration time is captured in https://discourse.llvm.org/t/rfc-hardening-in-libc/73925/23:

> Our current thinking is to only provide two ways of customizing verbose abort:
>
> A new way that would use __config_site.in and essentially allow a vendor to patch the code in __assert that defines the verbose abort function.
>
> The current way that allows redefining the macro at compile time (aimed at users).
> We’d prefer to not support link-time overriding. The reason is that we would like the default implementation to compile to a single instruction. The mechanism for link-time overriding would necessarily incur a function call that would be impossible to optimize out.

> What vendors are currently using configuration knob?

We are, and Chrome is too. There may be others, I'm not certain.

https://github.com/llvm/llvm-project/pull/93333