[libcxx-commits] [PATCH] D120719: [runtimes] Always configure libc++abi before libc++

Thu Mar 10 06:21:35 PST 2022

ldionne added a comment.

In D120719#3371499 <https://reviews.llvm.org/D120719#3371499>, @phosek wrote:

> In D120719#3371282 <https://reviews.llvm.org/D120719#3371282>, @ldionne wrote:
>
>> Gentle ping on this @phosek . I spoke to @beanz offline who said he was "neutral" on this change (I guess he doesn't quite like it but also won't push back). This isn't nice, but I think it's a necessary evil and it will allow greatly cleaning up how we pick up the ABI library from within libc++. Does this look OK to you?
>
> I'd like to do one additional experiment to see if there was a way to achieve this with generator expressions, I'll report back hopefully within a day if that's fine with you?

Yes, that's fine with me. My goal is to be able to land https://reviews.llvm.org/D120727. The way I test D120727 <https://reviews.llvm.org/D120727> right now is:

  cmake -S runtimes -B <build> -GNinja \
          -DCMAKE_BUILD_TYPE=Release \
          -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
          -DLIBCXX_ENABLE_STATIC_ABI_LIBRARY=ON \
          -DLIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY=ON \
          -DLIBCXX_STATICALLY_LINK_ABI_IN_STATIC_LIBRARY=ON

Without this patch, it fails due to `cxxabi_shared` not being a known target when we configure libcxx. If we apply this patch, it works. Also note that using `LIBCXX_ENABLE_STATIC_ABI_LIBRARY` & friends is relevant -- if you remove them, it's easy to implement D120727 <https://reviews.llvm.org/D120727> without needing this patch right here, you just have to make `libcxx-abi-static` be an `INTERFACE` library that links against `cxxabi_static` (even if it isn't defined yet). But that approach breaks when you use `LIBCXX_ENABLE_STATIC_ABI_LIBRARY` & friends, since we need to get `$<TARGET_LINKER_FILE:libcxx-abi-static>`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120719