Skip to content

Commit c06a8f9

Browse files
ldionnepetrhosek
ldionne
authored and
petrhosek
committed
[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
1 parent 3a6365a commit c06a8f9

File tree

11 files changed

+50
-92
lines changed

11 files changed

+50
-92
lines changed

‎libcxx/CMakeLists.txt

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -403,21 +403,23 @@ endif ()
403403
# Configure System
404404
#===============================================================================
405405

406+
# TODO: Projects that depend on libc++ should use LIBCXX_GENERATED_INCLUDE_DIR
407+
# instead of hard-coding include/c++/v1.
406408
if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
407409
set(LIBCXX_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
408-
set(LIBCXX_HEADER_DIR ${LLVM_BINARY_DIR})
410+
set(LIBCXX_GENERATED_INCLUDE_DIR "${LLVM_BINARY_DIR}/include/c++/v1")
409411
set(LIBCXX_INSTALL_LIBRARY_DIR lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++)
410412
if(LIBCXX_LIBDIR_SUBDIR)
411413
string(APPEND LIBCXX_LIBRARY_DIR /${LIBCXX_LIBDIR_SUBDIR})
412414
string(APPEND LIBCXX_INSTALL_LIBRARY_DIR /${LIBCXX_LIBDIR_SUBDIR})
413415
endif()
414416
elseif(LLVM_LIBRARY_OUTPUT_INTDIR)
415417
set(LIBCXX_LIBRARY_DIR ${LLVM_LIBRARY_OUTPUT_INTDIR})
416-
set(LIBCXX_HEADER_DIR ${LLVM_BINARY_DIR})
418+
set(LIBCXX_GENERATED_INCLUDE_DIR "${LLVM_BINARY_DIR}/include/c++/v1")
417419
set(LIBCXX_INSTALL_LIBRARY_DIR lib${LIBCXX_LIBDIR_SUFFIX})
418420
else()
419421
set(LIBCXX_LIBRARY_DIR ${CMAKE_BINARY_DIR}/lib${LIBCXX_LIBDIR_SUFFIX})
420-
set(LIBCXX_HEADER_DIR ${CMAKE_BINARY_DIR})
422+
set(LIBCXX_GENERATED_INCLUDE_DIR "${CMAKE_BINARY_DIR}/include/c++/v1")
421423
set(LIBCXX_INSTALL_LIBRARY_DIR lib${LIBCXX_LIBDIR_SUFFIX})
422424
endif()
423425

‎libcxx/benchmarks/CMakeLists.txt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ set(CMAKE_FOLDER "${CMAKE_FOLDER}/Benchmarks")
1010
set(BENCHMARK_LIBCXX_COMPILE_FLAGS
1111
-Wno-unused-command-line-argument
1212
-nostdinc++
13-
-isystem ${LIBCXX_SOURCE_DIR}/include
13+
-isystem "${LIBCXX_GENERATED_INCLUDE_DIR}"
1414
-L${LIBCXX_LIBRARY_DIR}
1515
-Wl,-rpath,${LIBCXX_LIBRARY_DIR}
1616
${SANITIZER_FLAGS}
@@ -20,7 +20,6 @@ if (DEFINED LIBCXX_CXX_ABI_LIBRARY_PATH)
2020
-L${LIBCXX_CXX_ABI_LIBRARY_PATH}
2121
-Wl,-rpath,${LIBCXX_CXX_ABI_LIBRARY_PATH})
2222
endif()
23-
list(APPEND BENCHMARK_LIBCXX_COMPILE_FLAGS -include "${LIBCXX_BINARY_DIR}/__config_site")
2423
split_list(BENCHMARK_LIBCXX_COMPILE_FLAGS)
2524

2625
ExternalProject_Add(google-benchmark-libcxx

‎libcxx/cmake/Modules/HandleLibCXXABI.cmake

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,14 @@ macro(setup_abi_lib abidefines abishared abistatic abifiles abidirs)
5252
COMMENT "Copying C++ ABI header ${fpath}...")
5353
list(APPEND abilib_headers "${dst}")
5454

55-
if (LIBCXX_HEADER_DIR)
56-
set(dst "${LIBCXX_HEADER_DIR}/include/c++/v1/${dstdir}/${fpath}")
57-
add_custom_command(OUTPUT ${dst}
58-
DEPENDS ${src}
59-
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
60-
COMMENT "Copying C++ ABI header ${fpath}...")
61-
list(APPEND abilib_headers "${dst}")
62-
endif()
55+
# TODO: libc++ shouldn't be responsible for copying the libc++abi
56+
# headers into the right location.
57+
set(dst "${LIBCXX_GENERATED_INCLUDE_DIR}/include/c++/v1/${dstdir}/${fpath}")
58+
add_custom_command(OUTPUT ${dst}
59+
DEPENDS ${src}
60+
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
61+
COMMENT "Copying C++ ABI header ${fpath}...")
62+
list(APPEND abilib_headers "${dst}")
6363

6464
if (LIBCXX_INSTALL_HEADERS)
6565
install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"

‎libcxx/docs/TestingLibcxx.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ whether the required libraries have been built, you can use the
3737
$ <build>/bin/llvm-lit -sv libcxx/test/std/depr/depr.c.headers/stdlib_h.pass.cpp # Run a single test
3838
$ <build>/bin/llvm-lit -sv libcxx/test/std/atomics libcxx/test/std/threads # Test std::thread and std::atomic
3939
40+
In the default configuration, the tests are built against headers that form a
41+
fake installation root of libc++. This installation root has to be updated when
42+
changes are made to the headers, so you should re-run the `cxx-test-depends`
43+
target before running the tests manually with `lit` when you make any sort of
44+
change, including to the headers.
45+
4046
Sometimes you'll want to change the way LIT is running the tests. Custom options
4147
can be specified using the `--param=<name>=<val>` flag. The most common option
4248
you'll want to change is the standard dialect (ie -std=c++XX). By default the

‎libcxx/include/CMakeLists.txt

Lines changed: 21 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ set(files
44
__bits
55
__bsd_locale_defaults.h
66
__bsd_locale_fallbacks.h
7+
__config
78
__debug
89
__errc
910
__functional_03
@@ -190,65 +191,28 @@ set(files
190191
wctype.h
191192
)
192193

193-
configure_file("__config_site.in"
194-
"${LIBCXX_BINARY_DIR}/__config_site"
195-
@ONLY)
194+
configure_file("__config_site.in" "${LIBCXX_GENERATED_INCLUDE_DIR}/__config_site" @ONLY)
196195

197-
# Generate a custom __config header. The new header is created
198-
# by prepending __config_site to the current __config header.
199-
add_custom_command(OUTPUT ${LIBCXX_BINARY_DIR}/__generated_config
200-
COMMAND ${Python3_EXECUTABLE} ${LIBCXX_SOURCE_DIR}/utils/cat_files.py
201-
${LIBCXX_BINARY_DIR}/__config_site
202-
${LIBCXX_SOURCE_DIR}/include/__config
203-
-o ${LIBCXX_BINARY_DIR}/__generated_config
204-
DEPENDS ${LIBCXX_SOURCE_DIR}/include/__config
205-
${LIBCXX_BINARY_DIR}/__config_site
206-
)
207-
# Add a target that executes the generation commands.
208-
add_custom_target(cxx-generated-config ALL
209-
DEPENDS ${LIBCXX_BINARY_DIR}/__generated_config)
210-
211-
if(LIBCXX_HEADER_DIR)
212-
set(output_dir ${LIBCXX_HEADER_DIR}/include/c++/v1)
213-
214-
set(out_files)
215-
foreach(f ${files})
216-
set(src ${CMAKE_CURRENT_SOURCE_DIR}/${f})
217-
set(dst ${output_dir}/${f})
218-
add_custom_command(OUTPUT ${dst}
219-
DEPENDS ${src}
220-
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
221-
COMMENT "Copying CXX header ${f}")
222-
list(APPEND out_files ${dst})
223-
endforeach()
224-
225-
# Copy the generated header as __config into build directory.
226-
set(src ${LIBCXX_BINARY_DIR}/__generated_config)
227-
set(dst ${output_dir}/__config)
196+
set(_all_includes "${LIBCXX_GENERATED_INCLUDE_DIR}/__config_site")
197+
foreach(f ${files})
198+
set(src "${CMAKE_CURRENT_SOURCE_DIR}/${f}")
199+
set(dst "${LIBCXX_GENERATED_INCLUDE_DIR}/${f}")
228200
add_custom_command(OUTPUT ${dst}
229-
DEPENDS ${src} cxx-generated-config
230-
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
231-
COMMENT "Copying CXX __config")
232-
list(APPEND out_files ${dst})
233-
add_custom_target(generate-cxx-headers ALL DEPENDS ${out_files})
201+
DEPENDS ${src}
202+
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
203+
COMMENT "Copying CXX header ${f}")
204+
list(APPEND _all_includes "${dst}")
205+
endforeach()
234206

235-
add_library(cxx-headers INTERFACE)
236-
add_dependencies(cxx-headers generate-cxx-headers ${LIBCXX_CXX_ABI_HEADER_TARGET})
237-
# TODO: Use target_include_directories once we figure out why that breaks the runtimes build
238-
if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC" OR "${CMAKE_CXX_SIMULATE_ID}" STREQUAL "MSVC")
239-
target_compile_options(cxx-headers INTERFACE /I "${output_dir}")
240-
else()
241-
target_compile_options(cxx-headers INTERFACE -I "${output_dir}")
242-
endif()
207+
add_custom_target(generate-cxx-headers DEPENDS ${_all_includes})
243208

244-
# Make sure the generated __config_site header is included when we build the library.
245-
if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC" OR "${CMAKE_CXX_SIMULATE_ID}" STREQUAL "MSVC")
246-
target_compile_options(cxx-headers INTERFACE /FI "${LIBCXX_BINARY_DIR}/__config_site")
247-
else()
248-
target_compile_options(cxx-headers INTERFACE -include "${LIBCXX_BINARY_DIR}/__config_site")
249-
endif()
209+
add_library(cxx-headers INTERFACE)
210+
add_dependencies(cxx-headers generate-cxx-headers ${LIBCXX_CXX_ABI_HEADER_TARGET})
211+
# TODO: Use target_include_directories once we figure out why that breaks the runtimes build
212+
if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC" OR "${CMAKE_CXX_SIMULATE_ID}" STREQUAL "MSVC")
213+
target_compile_options(cxx-headers INTERFACE /I "${LIBCXX_GENERATED_INCLUDE_DIR}")
250214
else()
251-
add_library(cxx-headers INTERFACE)
215+
target_compile_options(cxx-headers INTERFACE -I "${LIBCXX_GENERATED_INCLUDE_DIR}")
252216
endif()
253217

254218
if (LIBCXX_INSTALL_HEADERS)
@@ -261,16 +225,15 @@ if (LIBCXX_INSTALL_HEADERS)
261225
)
262226
endforeach()
263227

264-
# Install the generated header as __config.
265-
install(FILES ${LIBCXX_BINARY_DIR}/__generated_config
228+
# Install the generated __config_site.
229+
install(FILES ${LIBCXX_GENERATED_INCLUDE_DIR}/__config_site
266230
DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1
267231
PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ
268-
RENAME __config
269232
COMPONENT cxx-headers)
270233

271234
if (NOT CMAKE_CONFIGURATION_TYPES)
272235
add_custom_target(install-cxx-headers
273-
DEPENDS cxx-headers cxx-generated-config
236+
DEPENDS cxx-headers
274237
COMMAND "${CMAKE_COMMAND}"
275238
-DCMAKE_INSTALL_COMPONENT=cxx-headers
276239
-P "${CMAKE_BINARY_DIR}/cmake_install.cmake")

‎libcxx/include/__config

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
#ifndef _LIBCPP_CONFIG
1111
#define _LIBCPP_CONFIG
1212

13+
#include <__config_site>
14+
1315
#if defined(_MSC_VER) && !defined(__clang__)
1416
# if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
1517
# define _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER

‎libcxx/test/configs/legacy.cfg.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import os
44
import site
55

6+
config.cxx_headers = "@LIBCXX_GENERATED_INCLUDE_DIR@"
67
config.cxx_under_test = "@CMAKE_CXX_COMPILER@"
78
config.project_obj_root = "@CMAKE_BINARY_DIR@"
89
config.libcxx_src_root = "@LIBCXX_SOURCE_DIR@"

‎libcxx/utils/ci/run-buildbot

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,9 +399,12 @@ legacy-standalone)
399399
-DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
400400
-DLLVM_PATH="${MONOREPO_ROOT}/llvm" \
401401
-DLIBCXXABI_LIBCXX_PATH="${MONOREPO_ROOT}/libcxx" \
402-
-DLIBCXXABI_LIBCXX_INCLUDES="${MONOREPO_ROOT}/libcxx/include" \
402+
-DLIBCXXABI_LIBCXX_INCLUDES="${BUILD_DIR}/libcxx/include/c++/v1" \
403403
-DLIBCXXABI_LIBCXX_LIBRARY_PATH="${BUILD_DIR}/libcxx/lib"
404404

405+
echo "+++ Generating libc++ headers"
406+
${NINJA} -vC "${BUILD_DIR}/libcxx" generate-cxx-headers
407+
405408
echo "+++ Building libc++abi"
406409
${NINJA} -vC "${BUILD_DIR}/libcxxabi" cxxabi
407410

‎libcxx/utils/libcxx/test/config.py

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,6 @@ def configure_default_compile_flags(self):
334334

335335
def configure_compile_flags_header_includes(self):
336336
support_path = os.path.join(self.libcxx_src_root, 'test', 'support')
337-
self.configure_config_site_header()
338337
if self.cxx_stdlib_under_test != 'libstdc++' and \
339338
not self.target_info.is_windows() and \
340339
not self.target_info.is_zos():
@@ -352,33 +351,19 @@ def configure_compile_flags_header_includes(self):
352351
'set_windows_crt_report_mode.h')
353352
]
354353
cxx_headers = self.get_lit_conf('cxx_headers')
355-
if cxx_headers == '' or (cxx_headers is None
356-
and self.cxx_stdlib_under_test != 'libc++'):
354+
if cxx_headers is None and self.cxx_stdlib_under_test != 'libc++':
357355
self.lit_config.note('using the system cxx headers')
358356
return
359357
self.cxx.compile_flags += ['-nostdinc++']
360-
if cxx_headers is None:
361-
cxx_headers = os.path.join(self.libcxx_src_root, 'include')
362358
if not os.path.isdir(cxx_headers):
363-
self.lit_config.fatal("cxx_headers='%s' is not a directory."
364-
% cxx_headers)
359+
self.lit_config.fatal("cxx_headers='{}' is not a directory.".format(cxx_headers))
365360
self.cxx.compile_flags += ['-I' + cxx_headers]
366361
if self.libcxx_obj_root is not None:
367362
cxxabi_headers = os.path.join(self.libcxx_obj_root, 'include',
368363
'c++build')
369364
if os.path.isdir(cxxabi_headers):
370365
self.cxx.compile_flags += ['-I' + cxxabi_headers]
371366

372-
def configure_config_site_header(self):
373-
# Check for a possible __config_site in the build directory. We
374-
# use this if it exists.
375-
if self.libcxx_obj_root is None:
376-
return
377-
config_site_header = os.path.join(self.libcxx_obj_root, '__config_site')
378-
if not os.path.isfile(config_site_header):
379-
return
380-
self.cxx.compile_flags += ['-include', config_site_header]
381-
382367
def configure_link_flags(self):
383368
# Configure library path
384369
self.configure_link_flags_cxx_library_path()

‎libcxxabi/test/libcxxabi/test/config.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ def configure_compile_flags(self):
6060
super(Configuration, self).configure_compile_flags()
6161

6262
def configure_compile_flags_header_includes(self):
63-
self.configure_config_site_header()
6463
cxx_headers = self.get_lit_conf('cxx_headers', None) or \
6564
os.path.join(self.libcxxabi_hdr_root, 'include', 'c++', 'v1')
6665
if cxx_headers == '':

0 commit comments

Comments
 (0)