[libcxx-commits] [libcxxabi] c06a8f9 - [libc++] Include <__config_site> from <__config>
Petr Hosek via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Mar 30 14:06:26 PDT 2021
Author: Louis Dionne
Date: 2021-03-30T14:06:11-07:00
New Revision: c06a8f9caa51c7ea71dac716e0a35f5e343e4546
URL: https://github.com/llvm/llvm-project/commit/c06a8f9caa51c7ea71dac716e0a35f5e343e4546
DIFF: https://github.com/llvm/llvm-project/commit/c06a8f9caa51c7ea71dac716e0a35f5e343e4546.diff
LOG: [libc++] Include <__config_site> from <__config>
Prior to this patch, we would generate a fancy <__config> header by
concatenating <__config_site> and <__config>. This complexifies the
build system and also increases the difference between what's tested
and what's actually installed.
This patch removes that complexity and instead simply installs <__config_site>
alongside the libc++ headers. <__config_site> is then included by <__config>,
which is much simpler. Doing this also opens the door to having different
<__config_site> headers depending on the target, which was impossible before.
It does change the workflow for testing header-only changes to libc++.
Previously, we would run `lit` against the headers in libcxx/include.
After this patch, we run it against a fake installation root of the
headers (containing a proper <__config_site> header). This makes use
closer to testing what we actually install, which is good, however it
does mean that we have to update that root before testing header changes.
Thus, we now need to run `ninja check-cxx-deps` before running `lit` by
hand.
Differential Revision: https://reviews.llvm.org/D97572
Added:
Modified:
libcxx/CMakeLists.txt
libcxx/benchmarks/CMakeLists.txt
libcxx/cmake/Modules/HandleLibCXXABI.cmake
libcxx/docs/TestingLibcxx.rst
libcxx/include/CMakeLists.txt
libcxx/include/__config
libcxx/test/configs/legacy.cfg.in
libcxx/utils/ci/run-buildbot
libcxx/utils/libcxx/test/config.py
libcxxabi/test/libcxxabi/test/config.py
libunwind/test/libunwind/test/config.py
Removed:
################################################################################
diff --git a/libcxx/CMakeLists.txt b/libcxx/CMakeLists.txt
index 708405363500d..ffc1dad508509 100644
--- a/libcxx/CMakeLists.txt
+++ b/libcxx/CMakeLists.txt
@@ -403,9 +403,11 @@ endif ()
# Configure System
#===============================================================================
+# TODO: Projects that depend on libc++ should use LIBCXX_GENERATED_INCLUDE_DIR
+# instead of hard-coding include/c++/v1.
if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
set(LIBCXX_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
- set(LIBCXX_HEADER_DIR ${LLVM_BINARY_DIR})
+ set(LIBCXX_GENERATED_INCLUDE_DIR "${LLVM_BINARY_DIR}/include/c++/v1")
set(LIBCXX_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
if(LIBCXX_LIBDIR_SUBDIR)
string(APPEND LIBCXX_LIBRARY_DIR /${LIBCXX_LIBDIR_SUBDIR})
@@ -413,11 +415,11 @@ if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
endif()
elseif(LLVM_LIBRARY_OUTPUT_INTDIR)
set(LIBCXX_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
- set(LIBCXX_HEADER_DIR ${LLVM_BINARY_DIR})
+ set(LIBCXX_GENERATED_INCLUDE_DIR "${LLVM_BINARY_DIR}/include/c++/v1")
set(LIBCXX_INSTALL_LIBRARY_DIR lib${LIBCXX_LIBDIR_SUFFIX})
else()
set(LIBCXX_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBCXX_LIBDIR_SUFFIX})
- set(LIBCXX_HEADER_DIR ${CMAKE_BINARY_DIR})
+ set(LIBCXX_GENERATED_INCLUDE_DIR "${CMAKE_BINARY_DIR}/include/c++/v1")
set(LIBCXX_INSTALL_LIBRARY_DIR lib${LIBCXX_LIBDIR_SUFFIX})
endif()
diff --git a/libcxx/benchmarks/CMakeLists.txt b/libcxx/benchmarks/CMakeLists.txt
index 0327f05ccfa6d..47a463fd7cf66 100644
--- a/libcxx/benchmarks/CMakeLists.txt
+++ b/libcxx/benchmarks/CMakeLists.txt
@@ -10,7 +10,7 @@ set(CMAKE_FOLDER "${CMAKE_FOLDER}/Benchmarks")
set(BENCHMARK_LIBCXX_COMPILE_FLAGS
-Wno-unused-command-line-argument
-nostdinc++
- -isystem ${LIBCXX_SOURCE_DIR}/include
+ -isystem "${LIBCXX_GENERATED_INCLUDE_DIR}"
-L${LIBCXX_LIBRARY_DIR}
-Wl,-rpath,${LIBCXX_LIBRARY_DIR}
${SANITIZER_FLAGS}
@@ -20,7 +20,6 @@ if (DEFINED LIBCXX_CXX_ABI_LIBRARY_PATH)
-L${LIBCXX_CXX_ABI_LIBRARY_PATH}
-Wl,-rpath,${LIBCXX_CXX_ABI_LIBRARY_PATH})
endif()
-list(APPEND BENCHMARK_LIBCXX_COMPILE_FLAGS -include "${LIBCXX_BINARY_DIR}/__config_site")
split_list(BENCHMARK_LIBCXX_COMPILE_FLAGS)
ExternalProject_Add(google-benchmark-libcxx
diff --git a/libcxx/cmake/Modules/HandleLibCXXABI.cmake b/libcxx/cmake/Modules/HandleLibCXXABI.cmake
index 5d2764e870e99..fdd5c7ffe77d8 100644
--- a/libcxx/cmake/Modules/HandleLibCXXABI.cmake
+++ b/libcxx/cmake/Modules/HandleLibCXXABI.cmake
@@ -52,14 +52,14 @@ macro(setup_abi_lib abidefines abishared abistatic abifiles abidirs)
COMMENT "Copying C++ ABI header ${fpath}...")
list(APPEND abilib_headers "${dst}")
- if (LIBCXX_HEADER_DIR)
- set(dst "${LIBCXX_HEADER_DIR}/include/c++/v1/${dstdir}/${fpath}")
- add_custom_command(OUTPUT ${dst}
- DEPENDS ${src}
- COMMAND ${CMAKE_COMMAND} -E copy_if_
diff erent ${src} ${dst}
- COMMENT "Copying C++ ABI header ${fpath}...")
- list(APPEND abilib_headers "${dst}")
- endif()
+ # TODO: libc++ shouldn't be responsible for copying the libc++abi
+ # headers into the right location.
+ set(dst "${LIBCXX_GENERATED_INCLUDE_DIR}/include/c++/v1/${dstdir}/${fpath}")
+ add_custom_command(OUTPUT ${dst}
+ DEPENDS ${src}
+ COMMAND ${CMAKE_COMMAND} -E copy_if_
diff erent ${src} ${dst}
+ COMMENT "Copying C++ ABI header ${fpath}...")
+ list(APPEND abilib_headers "${dst}")
if (LIBCXX_INSTALL_HEADERS)
install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
diff --git a/libcxx/docs/TestingLibcxx.rst b/libcxx/docs/TestingLibcxx.rst
index ae014729c12f9..50aeb3e1b81d9 100644
--- a/libcxx/docs/TestingLibcxx.rst
+++ b/libcxx/docs/TestingLibcxx.rst
@@ -37,6 +37,12 @@ whether the required libraries have been built, you can use the
$ <build>/bin/llvm-lit -sv libcxx/test/std/depr/depr.c.headers/stdlib_h.pass.cpp # Run a single test
$ <build>/bin/llvm-lit -sv libcxx/test/std/atomics libcxx/test/std/threads # Test std::thread and std::atomic
+In the default configuration, the tests are built against headers that form a
+fake installation root of libc++. This installation root has to be updated when
+changes are made to the headers, so you should re-run the `cxx-test-depends`
+target before running the tests manually with `lit` when you make any sort of
+change, including to the headers.
+
Sometimes you'll want to change the way LIT is running the tests. Custom options
can be specified using the `--param=<name>=<val>` flag. The most common option
you'll want to change is the standard dialect (ie -std=c++XX). By default the
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index dd68e5f9e3cab..daf504e09298c 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -4,6 +4,7 @@ set(files
__bits
__bsd_locale_defaults.h
__bsd_locale_fallbacks.h
+ __config
__debug
__errc
__functional_03
@@ -190,65 +191,28 @@ set(files
wctype.h
)
-configure_file("__config_site.in"
- "${LIBCXX_BINARY_DIR}/__config_site"
- @ONLY)
+configure_file("__config_site.in" "${LIBCXX_GENERATED_INCLUDE_DIR}/__config_site" @ONLY)
-# Generate a custom __config header. The new header is created
-# by prepending __config_site to the current __config header.
-add_custom_command(OUTPUT ${LIBCXX_BINARY_DIR}/__generated_config
- COMMAND ${Python3_EXECUTABLE} ${LIBCXX_SOURCE_DIR}/utils/cat_files.py
- ${LIBCXX_BINARY_DIR}/__config_site
- ${LIBCXX_SOURCE_DIR}/include/__config
- -o ${LIBCXX_BINARY_DIR}/__generated_config
- DEPENDS ${LIBCXX_SOURCE_DIR}/include/__config
- ${LIBCXX_BINARY_DIR}/__config_site
-)
-# Add a target that executes the generation commands.
-add_custom_target(cxx-generated-config ALL
- DEPENDS ${LIBCXX_BINARY_DIR}/__generated_config)
-
-if(LIBCXX_HEADER_DIR)
- set(output_dir ${LIBCXX_HEADER_DIR}/include/c++/v1)
-
- set(out_files)
- foreach(f ${files})
- set(src ${CMAKE_CURRENT_SOURCE_DIR}/${f})
- set(dst ${output_dir}/${f})
- add_custom_command(OUTPUT ${dst}
- DEPENDS ${src}
- COMMAND ${CMAKE_COMMAND} -E copy_if_
diff erent ${src} ${dst}
- COMMENT "Copying CXX header ${f}")
- list(APPEND out_files ${dst})
- endforeach()
-
- # Copy the generated header as __config into build directory.
- set(src ${LIBCXX_BINARY_DIR}/__generated_config)
- set(dst ${output_dir}/__config)
+set(_all_includes "${LIBCXX_GENERATED_INCLUDE_DIR}/__config_site")
+foreach(f ${files})
+ set(src "${CMAKE_CURRENT_SOURCE_DIR}/${f}")
+ set(dst "${LIBCXX_GENERATED_INCLUDE_DIR}/${f}")
add_custom_command(OUTPUT ${dst}
- DEPENDS ${src} cxx-generated-config
- COMMAND ${CMAKE_COMMAND} -E copy_if_
diff erent ${src} ${dst}
- COMMENT "Copying CXX __config")
- list(APPEND out_files ${dst})
- add_custom_target(generate-cxx-headers ALL DEPENDS ${out_files})
+ DEPENDS ${src}
+ COMMAND ${CMAKE_COMMAND} -E copy_if_
diff erent ${src} ${dst}
+ COMMENT "Copying CXX header ${f}")
+ list(APPEND _all_includes "${dst}")
+endforeach()
- add_library(cxx-headers INTERFACE)
- add_dependencies(cxx-headers generate-cxx-headers ${LIBCXX_CXX_ABI_HEADER_TARGET})
- # 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 "${output_dir}")
- else()
- target_compile_options(cxx-headers INTERFACE -I "${output_dir}")
- endif()
+add_custom_target(generate-cxx-headers DEPENDS ${_all_includes})
- # Make sure the generated __config_site header is included when we build the library.
- if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC" OR "${CMAKE_CXX_SIMULATE_ID}" STREQUAL "MSVC")
- target_compile_options(cxx-headers INTERFACE /FI "${LIBCXX_BINARY_DIR}/__config_site")
- else()
- target_compile_options(cxx-headers INTERFACE -include "${LIBCXX_BINARY_DIR}/__config_site")
- endif()
+add_library(cxx-headers INTERFACE)
+add_dependencies(cxx-headers generate-cxx-headers ${LIBCXX_CXX_ABI_HEADER_TARGET})
+# 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()
- add_library(cxx-headers INTERFACE)
+ target_compile_options(cxx-headers INTERFACE -I "${LIBCXX_GENERATED_INCLUDE_DIR}")
endif()
if (LIBCXX_INSTALL_HEADERS)
@@ -261,16 +225,15 @@ if (LIBCXX_INSTALL_HEADERS)
)
endforeach()
- # Install the generated header as __config.
- install(FILES ${LIBCXX_BINARY_DIR}/__generated_config
+ # Install the generated __config_site.
+ install(FILES ${LIBCXX_GENERATED_INCLUDE_DIR}/__config_site
DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1
PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
- RENAME __config
COMPONENT cxx-headers)
if (NOT CMAKE_CONFIGURATION_TYPES)
add_custom_target(install-cxx-headers
- DEPENDS cxx-headers cxx-generated-config
+ DEPENDS cxx-headers
COMMAND "${CMAKE_COMMAND}"
-DCMAKE_INSTALL_COMPONENT=cxx-headers
-P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
diff --git a/libcxx/include/__config b/libcxx/include/__config
index cd748fde25863..635730402afaa 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -10,6 +10,8 @@
#ifndef _LIBCPP_CONFIG
#define _LIBCPP_CONFIG
+#include <__config_site>
+
#if defined(_MSC_VER) && !defined(__clang__)
# if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# define _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER
diff --git a/libcxx/test/configs/legacy.cfg.in b/libcxx/test/configs/legacy.cfg.in
index 363ab3364efce..13cc35def4568 100644
--- a/libcxx/test/configs/legacy.cfg.in
+++ b/libcxx/test/configs/legacy.cfg.in
@@ -3,6 +3,7 @@
import os
import site
+config.cxx_headers = "@LIBCXX_GENERATED_INCLUDE_DIR@"
config.cxx_under_test = "@CMAKE_CXX_COMPILER@"
config.project_obj_root = "@CMAKE_BINARY_DIR@"
config.libcxx_src_root = "@LIBCXX_SOURCE_DIR@"
diff --git a/libcxx/utils/ci/run-buildbot b/libcxx/utils/ci/run-buildbot
index 0b677cd7ec0ee..fcdc1258fa8a8 100755
--- a/libcxx/utils/ci/run-buildbot
+++ b/libcxx/utils/ci/run-buildbot
@@ -399,9 +399,12 @@ legacy-standalone)
-DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
-DLLVM_PATH="${MONOREPO_ROOT}/llvm" \
-DLIBCXXABI_LIBCXX_PATH="${MONOREPO_ROOT}/libcxx" \
- -DLIBCXXABI_LIBCXX_INCLUDES="${MONOREPO_ROOT}/libcxx/include" \
+ -DLIBCXXABI_LIBCXX_INCLUDES="${BUILD_DIR}/libcxx/include/c++/v1" \
-DLIBCXXABI_LIBCXX_LIBRARY_PATH="${BUILD_DIR}/libcxx/lib"
+ echo "+++ Generating libc++ headers"
+ ${NINJA} -vC "${BUILD_DIR}/libcxx" generate-cxx-headers
+
echo "+++ Building libc++abi"
${NINJA} -vC "${BUILD_DIR}/libcxxabi" cxxabi
diff --git a/libcxx/utils/libcxx/test/config.py b/libcxx/utils/libcxx/test/config.py
index 3262c37f21535..88527a49b686f 100644
--- a/libcxx/utils/libcxx/test/config.py
+++ b/libcxx/utils/libcxx/test/config.py
@@ -334,7 +334,6 @@ def configure_default_compile_flags(self):
def configure_compile_flags_header_includes(self):
support_path = os.path.join(self.libcxx_src_root, 'test', 'support')
- self.configure_config_site_header()
if self.cxx_stdlib_under_test != 'libstdc++' and \
not self.target_info.is_windows() and \
not self.target_info.is_zos():
@@ -352,16 +351,12 @@ def configure_compile_flags_header_includes(self):
'set_windows_crt_report_mode.h')
]
cxx_headers = self.get_lit_conf('cxx_headers')
- if cxx_headers == '' or (cxx_headers is None
- and self.cxx_stdlib_under_test != 'libc++'):
+ if cxx_headers is None and self.cxx_stdlib_under_test != 'libc++':
self.lit_config.note('using the system cxx headers')
return
self.cxx.compile_flags += ['-nostdinc++']
- if cxx_headers is None:
- cxx_headers = os.path.join(self.libcxx_src_root, 'include')
if not os.path.isdir(cxx_headers):
- self.lit_config.fatal("cxx_headers='%s' is not a directory."
- % cxx_headers)
+ self.lit_config.fatal("cxx_headers='{}' is not a directory.".format(cxx_headers))
self.cxx.compile_flags += ['-I' + cxx_headers]
if self.libcxx_obj_root is not None:
cxxabi_headers = os.path.join(self.libcxx_obj_root, 'include',
@@ -369,16 +364,6 @@ def configure_compile_flags_header_includes(self):
if os.path.isdir(cxxabi_headers):
self.cxx.compile_flags += ['-I' + cxxabi_headers]
- def configure_config_site_header(self):
- # Check for a possible __config_site in the build directory. We
- # use this if it exists.
- if self.libcxx_obj_root is None:
- return
- config_site_header = os.path.join(self.libcxx_obj_root, '__config_site')
- if not os.path.isfile(config_site_header):
- return
- self.cxx.compile_flags += ['-include', config_site_header]
-
def configure_link_flags(self):
# Configure library path
self.configure_link_flags_cxx_library_path()
diff --git a/libcxxabi/test/libcxxabi/test/config.py b/libcxxabi/test/libcxxabi/test/config.py
index 5357bb18b931c..f8f9927c8c8e9 100644
--- a/libcxxabi/test/libcxxabi/test/config.py
+++ b/libcxxabi/test/libcxxabi/test/config.py
@@ -60,7 +60,6 @@ def configure_compile_flags(self):
super(Configuration, self).configure_compile_flags()
def configure_compile_flags_header_includes(self):
- self.configure_config_site_header()
cxx_headers = self.get_lit_conf('cxx_headers', None) or \
os.path.join(self.libcxxabi_hdr_root, 'include', 'c++', 'v1')
if cxx_headers == '':
diff --git a/libunwind/test/libunwind/test/config.py b/libunwind/test/libunwind/test/config.py
index be2dcbd1af598..e3e1dfcaed88a 100644
--- a/libunwind/test/libunwind/test/config.py
+++ b/libunwind/test/libunwind/test/config.py
@@ -52,8 +52,6 @@ def configure_compile_flags(self):
super(Configuration, self).configure_compile_flags()
def configure_compile_flags_header_includes(self):
- self.configure_config_site_header()
-
libunwind_headers = self.get_lit_conf(
'libunwind_headers',
os.path.join(self.libunwind_src_root, 'include'))
More information about the libcxx-commits
mailing list