[libcxx-commits] [PATCH] D97314: [libc++abi] Link against cxx-headers when available
Shoaib Meenai via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Mar 2 17:27:06 PST 2021
smeenai added a comment.
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97314/new/
https://reviews.llvm.org/D97314
More information about the libcxx-commits
mailing list