[libc++abi] Install the libc++abi headers from libc++abi

[ldionne]

Summary

libc++abi should be responsible for installing its own headers, it
doesn't make sense for libc++ to be responsible for it.

[ldionne]

llvm/utils/gn/secondary/libcxx/include/BUILD.gn

271–281

Trying to save you a headache @thakis, but I'd like you to double-check this is OK.

[phosek]

LGTM

libcxx/cmake/Modules/HandleLibCXXABI.cmake

15

We should also consider inlining this macro, I don't think it's getting us anything and I always have to lookup what each argument does so at least in my experience it harms readability.

[thakis]

Makes sense to me.

Just to confirm: Even back when clang assumed that libc++ headers shipped with the compiler instead of the sdk on macOS (which I think is still the case in currently shipping xcode), it didn't assume that the libc++abi headers ship with the compiler, right? (I patched this in and clang still seems to compile a program that includes <iostream> fine, so it looks like it.)

Is there anything else that installs this header? We don't need it, but someone might?

[ldionne]

In D101458#2723626, @thakis wrote:

Makes sense to me.

Just to confirm: Even back when clang assumed that libc++ headers shipped with the compiler instead of the sdk on macOS (which I think is still the case in currently shipping xcode), it didn't assume that the libc++abi headers ship with the compiler, right? (I patched this in and clang still seems to compile a program that includes <iostream> fine, so it looks like it.)

Fact: Clang used to assume that libc++ was shipped in <toolchain>/include/c++/v1, and nowadays it looks in <sdk>/include/c++/v1 first, and only falls back to <toolchain>/include/c++/v1.
Fact: Recent SDKs have the libc++ headers in <sdk>/usr/include/c++/v1.
Fact: The <cxxabi.h> header is present in both <sdk>/usr/include/c++/v1 and <sdk>/usr/include (and also in <toolchain>/usr/include/c++/v1 until we remove all traces of libc++/libc++abi from the toolchain). It looks like they are always the same thing. It's harmless but silly, since we should really have only one copy. It turns out I don't understand how the copy in <sdk>/usr/include ends up there, so I'll investigate that.

Your main question: I'm pretty sure Clang used to expect <cxxabi.h> in <toolchain>/usr/include/c++/v1, alongside the rest of libc++.

I actually hadn't realized that, and it does change what I was going to do. Instead of always installing <cxxabi.h> to <install>/usr/include, I think the correct thing to do would be to always install it to <install>/usr/include/c++/v1, but to do that directly from libc++abi. My next update will reflect that.

Edit I just found out where <sdk>/usr/include/cxxabi.h comes from. We copy it explicitly here: https://github.com/llvm/llvm-project/blob/main/libcxx/utils/ci/apple-install-libcxx.sh#L159. I wrote that code, I should have known!

libcxx/cmake/Modules/HandleLibCXXABI.cmake

15

Oh yeah, totally, I want to clean up this mess, one step at a time.

[ldionne]

Install <cxxabi.h> from libc++abi.

Pinging @dim since I think you folks use a different ABI library - you should make
sure that your ABI library is installing <cxxabi.h> in a place where users can find
it on its own.

[ldionne]

Drop the gn changes.

llvm/utils/gn/secondary/libcxx/include/BUILD.gn

271–281

@thakis I suspect you'll have more fixing up to do since you need to install the libc++abi headers from the libc++abi build. I'll just drop this GN change to avoid muddying the waters, but I'm still giving you this heads up.

[ldionne]

Link cxx_static against libc++abi to get the header dependency.

[ldionne]

Rebase onto main, and don't touch any logic inside libc++. In other words, only start installing
the headers from libc++abi. I'll keep any potentially-breaking change in a separate patch.

[ldionne]

Rebase onto main -- I am unable to reproduce the CI issue with GCC, and it looks like a fluke to me.

libcxx/cmake/Modules/HandleLibCXXABI.cmake

 
 #===============================================================================
 # Add an ABI library if appropriate
 #===============================================================================
 
 #
 # _setup_abi: Set up the build to use an ABI library
 #
 # Parameters:
 #   abidefines: A list of defines needed to compile libc++ with the ABI library
 #   abishared : The shared ABI library to link against.
 #   abistatic : The static ABI library to link against.
-#   abifiles  : A list of files (which may be relative paths) to copy into the
-#               libc++ build tree for the build.  These files will be copied
-#               twice: once into include/, so the libc++ build itself can find
-#               them, and once into include/c++/v1, so that a clang built into
-#               the same build area will find them.  These files will also be
-#               installed alongside the libc++ headers.
-#   abidirs   : A list of relative paths to create under an include directory
-#               in the libc++ build directory.
 #
 
 macro(setup_abi_lib abidefines abishared abistatic abifiles abidirs)
   list(APPEND LIBCXX_COMPILE_FLAGS ${abidefines})
   set(LIBCXX_CXX_ABI_INCLUDE_PATHS "${LIBCXX_CXX_ABI_INCLUDE_PATHS}"
     CACHE PATH
     "Paths to C++ ABI header directories separated by ';'." FORCE
     )
   set(LIBCXX_CXX_ABI_LIBRARY_PATH "${LIBCXX_CXX_ABI_LIBRARY_PATH}"
     CACHE PATH
     "Paths to C++ ABI library directory"
     )
   set(LIBCXX_CXX_SHARED_ABI_LIBRARY ${abishared})
   set(LIBCXX_CXX_STATIC_ABI_LIBRARY ${abistatic})
-  set(LIBCXX_ABILIB_FILES ${abifiles})
-
-  foreach(fpath ${LIBCXX_ABILIB_FILES})
-    set(found FALSE)
-    foreach(incpath ${LIBCXX_CXX_ABI_INCLUDE_PATHS})
-      message(STATUS "Looking for ${fpath} in ${incpath}")
-      if (EXISTS "${incpath}/${fpath}")
-        set(found TRUE)
-        message(STATUS "Looking for ${fpath} in ${incpath} - found")
-        get_filename_component(dstdir ${fpath} PATH)
-        get_filename_component(ifile ${fpath} NAME)
-        set(src ${incpath}/${fpath})
-
-        set(dst ${LIBCXX_BINARY_INCLUDE_DIR}/${dstdir}/${ifile})
-        add_custom_command(OUTPUT ${dst}
-            DEPENDS ${src}
-            COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
-            COMMENT "Copying C++ ABI header ${fpath}...")
-        list(APPEND abilib_headers "${dst}")
-
-        # TODO: libc++ shouldn't be responsible for copying the libc++abi
-        # headers into the right location.
-        set(dst "${LIBCXX_GENERATED_INCLUDE_DIR}/${dstdir}/${fpath}")
-        add_custom_command(OUTPUT ${dst}
-            DEPENDS ${src}
-            COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
-            COMMENT "Copying C++ ABI header ${fpath}...")
-        list(APPEND abilib_headers "${dst}")
-
-        if (LIBCXX_INSTALL_HEADERS)
-          install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
-            DESTINATION include/c++/v1/${dstdir}
-            COMPONENT cxx-headers
-            PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
-            )
-        endif()
-      else()
-        message(STATUS "Looking for ${fpath} in ${incpath} - not found")
-      endif()
-    endforeach()
-    if (NOT found)
-      message(WARNING "Failed to find ${fpath} in ${LIBCXX_CXX_ABI_INCLUDE_PATHS}")
-    endif()
-  endforeach()
-
-  include_directories("${LIBCXX_BINARY_INCLUDE_DIR}")
-  add_custom_target(cxx_abi_headers ALL DEPENDS ${abilib_headers})
-  set(LIBCXX_CXX_ABI_HEADER_TARGET "cxx_abi_headers")
 endmacro()
 
 
 # Configure based on the selected ABI library.
 if ("${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libstdc++" OR
     "${LIBCXX_CXX_ABI_LIBNAME}" STREQUAL "libsupc++")
   set(_LIBSUPCXX_INCLUDE_FILES
     cxxabi.h bits/c++config.h bits/os_defines.h bits/cpu_defines.h

libcxx/include/CMakeLists.txt

add_custom_command(OUTPUT ${dst}
     COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
     COMMENT "Copying CXX header ${f}")
   list(APPEND _all_includes "${dst}")
 endforeach()
 
 add_custom_target(generate-cxx-headers ALL DEPENDS ${_all_includes})
 
 add_library(cxx-headers INTERFACE)
-add_dependencies(cxx-headers generate-cxx-headers ${LIBCXX_CXX_ABI_HEADER_TARGET})
+add_dependencies(cxx-headers generate-cxx-headers)
 # TODO: Use target_include_directories once we figure out why that breaks the runtimes build
 if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC" OR "${CMAKE_CXX_SIMULATE_ID}" STREQUAL "MSVC")
   target_compile_options(cxx-headers INTERFACE /I "${LIBCXX_GENERATED_INCLUDE_DIR}")
 else()
   target_compile_options(cxx-headers INTERFACE -I "${LIBCXX_GENERATED_INCLUDE_DIR}")
 endif()
 
 if (LIBCXX_INSTALL_HEADERS)

libcxx/src/CMakeLists.txt

if (APPLE AND LLVM_USE_SANITIZER)
   endif()
 endif()
 
 if (LIBCXX_ENABLE_PARALLEL_ALGORITHMS AND NOT TARGET pstl::ParallelSTL)
   message(FATAL_ERROR "Could not find ParallelSTL")
 endif()
 
 function(cxx_set_common_defines name)
-  if(LIBCXX_CXX_ABI_HEADER_TARGET)
-    add_dependencies(${name} ${LIBCXX_CXX_ABI_HEADER_TARGET})
-  endif()
-
   if (LIBCXX_ENABLE_PARALLEL_ALGORITHMS)
     target_link_libraries(${name} PUBLIC pstl::ParallelSTL)
   endif()
 endfunction()
 
 split_list(LIBCXX_COMPILE_FLAGS)
 split_list(LIBCXX_LINK_FLAGS)
 

llvm/utils/gn/secondary/libcxx/include/BUILD.gn

sources = [
       "valarray",
       "variant",
       "vector",
       "version",
       "wchar.h",
       "wctype.h",
     ]
     deps = [ ":write_config_site" ]
-    if (target_os != "mac" && target_os != "win") {
-      # libcxx/cmake/Modules/HandleLibCXXABI.cmake sets
-      # LIBCXX_CXX_ABI_HEADER_TARGET if the libcxx abi library either of
-      # "libstdc++", "libsupc++", "libcxxabi", "libcxxrt", but not if it's "none",
-      # "default", or "vcruntime". So on Windows, these don't get copied due to
-      # LIBCXX_CXX_ABI_HEADER_TARGET not being set.
-      # On macOS, libcxx/CMakeLists.txt sets LIBCXX_CXX_ABI_SYSTEM to 1, which
-      # causes an empty header list to be passed to setup_abi_lib, so these
-      # don't get copied on macOS due to that.
-      deps += [ "//libcxxabi/include" ]
-    }
     outputs = [ "$root_build_dir/include/c++/v1/{{source_target_relative}}" ]
   }
 }
 
 config("include_config") {
   include_dirs = [ libcxx_generated_include_dir ]
 }
 
 group("include") {
   deps = [ ":copy_headers($default_toolchain)" ]
   public_configs = [ ":include_config" ]
 }