[libcxx-commits] [PATCH] D91574: [libc++] Simplify how we pick the typeinfo comparison

Tue Nov 17 09:39:10 PST 2020

smeenai added a comment.

In D91574#2400066 <https://reviews.llvm.org/D91574#2400066>, @ldionne wrote:

> In D91574#2398507 <https://reviews.llvm.org/D91574#2398507>, @smeenai wrote:
>
>> We have a large internal codebase that I migrated from libstdc++ to libc++. libstdc++ uses relaxed typeinfo comparisons, and I found that just naively switching to libc++ produced hundreds of exception-related issues. Configuring libc++ to use the non-unique implementation was by far the easiest way to get this codebase working. I'm aware that in most cases, there is a proper fix for the exception issues, but (a) it's really hard to justify the large time investment required to fix all the different issues, and (b) there's some cases involving `dlopen` and `RTLD_LOCAL` that I believe are impossible to get right with the unique implementation (and I think that was the motivation for libstdc++ to switch to relaxed typeinfo comparisons).
>>
>> Getting rid of the configurability seems problematic at least for ELF, where there's good reasons to want to use either the unique or non-unique implementations.
>
> Interesting, thanks for the information. This is the sort of feedback I was looking for when putting up this review.
>
> In that case, what I'll do instead is clarify that `LIBCXX_TYPEINFO_COMPARISON_IMPLEMENTATION` allows unconditionally overriding the implementation of typeinfo used, and have the non unique ARM implementation be `_LIBCPP_TYPEINFO_COMPARISON_IMPLEMENTATION == 3` like in this patch. It'll get rid of most of the confusing-ness while still letting you override it.

Sounds great, thank you!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91574