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

[ldionne]

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.

[phosek]

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?

[ldionne]

In 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. 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?

[phosek]

In D97314#2588445, @ldionne wrote:

In 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. 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?

[smeenai]

In D97314#2588702, @phosek wrote:

In D97314#2588445, @ldionne wrote:

In 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. 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.

[phosek]

In D97314#2598975, @smeenai wrote:

In D97314#2588702, @phosek wrote:

In D97314#2588445, @ldionne wrote:

In 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. 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]

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

libcxxabi/src/CMakeLists.txt

 if (MSVC_IDE OR XCODE)
   # Force them all into the headers dir on MSVC, otherwise they end up at
   # project scope because they don't have extensions.
   if (MSVC_IDE)
     source_group("Header Files" FILES ${LIBCXXABI_HEADERS})
   endif()
 endif()
 
-include_directories("${LIBCXXABI_LIBCXX_INCLUDES}")
-
 if (LIBCXXABI_HAS_CXA_THREAD_ATEXIT_IMPL)
   add_definitions(-DHAVE___CXA_THREAD_ATEXIT_IMPL)
 endif()
 
 if (LIBCXXABI_ENABLE_FORGIVING_DYNAMIC_CAST)
   add_definitions(-D_LIBCXXABI_FORGIVING_DYNAMIC_CAST)
 endif()
 

 if (NOT TARGET pstl::ParallelSTL)
   message(STATUS "Could not find ParallelSTL, libc++abi will not attempt to use it but the build may fail if the libc++ in use needs it to be available.")
 endif()
 
 # Build the shared library.
 if (LIBCXXABI_ENABLE_SHARED)
   add_library(cxxabi_shared SHARED ${LIBCXXABI_SOURCES} ${LIBCXXABI_HEADERS})
   target_link_libraries(cxxabi_shared PRIVATE ${LIBCXXABI_SHARED_LIBRARIES} ${LIBCXXABI_LIBRARIES})
+  if (TARGET cxx-headers)
+    target_link_libraries(cxxabi_shared PRIVATE cxx-headers)
+  else()
+    target_include_directories(cxxabi_shared PRIVATE "${LIBCXXABI_LIBCXX_INCLUDES}")
+  endif()
   if (TARGET pstl::ParallelSTL)
     target_link_libraries(cxxabi_shared PUBLIC pstl::ParallelSTL)
   endif()
   set_target_properties(cxxabi_shared
                         PROPERTIES
                           CXX_EXTENSIONS
                             OFF
                           CXX_STANDARD
if (APPLE)
     endif()
   endif()
 endif()
 
 # Build the static library.
 if (LIBCXXABI_ENABLE_STATIC)
   add_library(cxxabi_static STATIC ${LIBCXXABI_SOURCES} ${LIBCXXABI_HEADERS})
   target_link_libraries(cxxabi_static PRIVATE ${LIBCXXABI_STATIC_LIBRARIES} ${LIBCXXABI_LIBRARIES})
+  if (TARGET cxx-headers)
+    target_link_libraries(cxxabi_static PRIVATE cxx-headers)
+  else()
+    target_include_directories(cxxabi_static PRIVATE "${LIBCXXABI_LIBCXX_INCLUDES}")
+  endif()
   if (TARGET pstl::ParallelSTL)
     target_link_libraries(cxxabi_static PUBLIC pstl::ParallelSTL)
   endif()
   set_target_properties(cxxabi_static
                         PROPERTIES
                           CXX_EXTENSIONS
                             OFF
                           CXX_STANDARD