Page MenuHomePhabricator

[libc++abi] Link against cxx-headers when available
AbandonedPublic

Authored by ldionne on Feb 23 2021, 11:38 AM.

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

Using the cxx-headers target created by libc++ is better than using
LIBCXXABI_LIBCXX_INCLUDES since it propagates other things like the
include of the __config_site header, which is necessary to build
successfully in certain configurations.

We should in fact get rid of LIBCXXABI_LIBCXX_INCLUDES, however some
clients using the old standalone build still need it.

Diff Detail

Event Timeline

ldionne created this revision.Feb 23 2021, 11:38 AM
ldionne requested review of this revision.Feb 23 2021, 11:38 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 23 2021, 11:38 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

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. Ideally we would use generator expressions but they don't support dependencies. 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). @smeenai, @beanz, @compnerd do you have any thoughts on this/other ideas?

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. Ideally we would use generator expressions but they don't support dependencies. 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). @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?

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. Ideally we would use generator expressions but they don't support dependencies. 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). @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 for that purpose?

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. Ideally we would use generator expressions but they don't support dependencies. 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). @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 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, 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.

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. Ideally we would use generator expressions but they don't support dependencies. 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). @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 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, 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.

ldionne abandoned this revision.Thu, Apr 8, 1:15 PM

Abandoning since this was superseded by https://reviews.llvm.org/D98367.