[libcxx-commits] [PATCH] D97314: [libc++abi] Link against cxx-headers when available

[Petr Hosek via Phabricator via libcxx-commits]

phosek added a comment.

In D97314#2598975 <https://reviews.llvm.org/D97314#2598975>, @smeenai wrote:

> In D97314#2588702 <https://reviews.llvm.org/D97314#2588702>, @phosek wrote:
>
>> In D97314#2588445 <https://reviews.llvm.org/D97314#2588445>, @ldionne wrote:
>>
>>> In D97314#2585569 <https://reviews.llvm.org/D97314#2585569>, @phosek wrote:
>>>
>>>> I'm a bit worried about using `TARGET cxx-headers`. Whether that evaluates to `TRUE` or `FALSE` depends on the order of target evaluation, which in this case is determined by the order in which you put projects into `LLVM_ENABLE_PROJECTS`/`LLVM_ENABLE_RUNTIMES`. I ran into this recently in D97256 <https://reviews.llvm.org/D97256>. Ideally we would use generator expressions but they don't support dependencies <https://gitlab.kitware.com/cmake/cmake/-/issues/19467>. Alternative is to either use patterns like `"libcxx" IN_LIST LLVM_ENABLE_RUNTIMES` or canonicalizing the order in which we process targets (which we already do in some cases <https://github.com/llvm/llvm-project/blob/c2487bf7dfdda59b775b3d5a06684af243790125/runtimes/CMakeLists.txt#L57>). @smeenai, @beanz, @compnerd do you have any thoughts on this/other ideas?
>>>
>>> Could we create those targets upfront regardless of the order projects are included in? And then we'd populate those targets later on in their respective CMake files, but at least they would be visible globally. Kind of like introducing a forward-declaration in C++ to break circular dependencies.
>>>
>>> Would that make sense?
>>
>> That's an interesting idea, I haven't thought of that but I like it. The question is where would those targets get defined. Could we use the top-level runtimes build <https://github.com/llvm/llvm-project/blob/main/runtimes/CMakeLists.txt> for that purpose?
>
> How would we create the target? I don't think we can change the target type later, so e.g. we couldn't do a generic `add_custom_target` initially and then change it to an `add_library` later. We can do empty `add_library` calls starting in 3.11 (so we're good there), but we'd have to adjust projects to use `target_sources` instead of doing their own `add_library` calls.
>
> For `LLVM_ENABLE_PROJECTS`, we explicitly evaluate libcxxabi before libcxx <https://github.com/llvm/llvm-project/blob/a189b3b9e8bb398d9fe8770956f8ad1d58c2a214/llvm/projects/CMakeLists.txt#L35-L36>, and for `LLVM_ENABLE_RUNTIMES`, I believe we'll evaluate libc++ and libc++abi according to their order in that list, so yeah, I think the `TARGET cxx-headers` will be a problem here.

Using empty `add_library` to create the target and `target_sources` to specify sources sounds good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97314