From ac1ac0b3452a0fa5f856be5ed70be96fcd67bd7a Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Mon, 7 Apr 2025 13:59:03 -0400 Subject: [PATCH 1/4] [libc++] Make __config_site modular This patch makes the __config_site header modular, which solves various problems with non-modular headers. This requires going back to generating the modulemap file, since we only know how to make __config_site modular when we're not using the per-target runtime dir. The patch also adds a test that we support -Wnon-modular-include-in-module, which warns about non-modular includes from modules. --- libcxx/docs/Contributing.rst | 2 +- libcxx/include/CMakeLists.txt | 17 ++++++++++++++-- .../{module.modulemap => module.modulemap.in} | 1 + ...modular_include_in_module.compile.pass.cpp | 20 +++++++++++++++++++ libcxx/test/libcxx/lint/lint_headers.sh.py | 2 +- libcxx/utils/libcxx/header_information.py | 2 +- 6 files changed, 39 insertions(+), 5 deletions(-) rename libcxx/include/{module.modulemap => module.modulemap.in} (99%) create mode 100644 libcxx/test/libcxx/Wnon_modular_include_in_module.compile.pass.cpp diff --git a/libcxx/docs/Contributing.rst b/libcxx/docs/Contributing.rst index 5bcd6e3a7f03e..6aaa70764c2fa 100644 --- a/libcxx/docs/Contributing.rst +++ b/libcxx/docs/Contributing.rst @@ -116,7 +116,7 @@ sure you don't forget anything: - Did you add all new named declarations to the ``std`` module? - If you added a header: - - Did you add it to ``include/module.modulemap``? + - Did you add it to ``include/module.modulemap.in``? - Did you add it to ``include/CMakeLists.txt``? - If it's a public header, did you update ``utils/libcxx/header_information.py``? diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt index 4431582cc33ca..f61b6ac6ad94f 100644 --- a/libcxx/include/CMakeLists.txt +++ b/libcxx/include/CMakeLists.txt @@ -1027,7 +1027,6 @@ set(files mdspan memory memory_resource - module.modulemap mutex new numbers @@ -1667,8 +1666,16 @@ set(files configure_file("__config_site.in" "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__config_site" @ONLY) configure_file("${LIBCXX_ASSERTION_HANDLER_FILE}" "${LIBCXX_GENERATED_INCLUDE_DIR}/__assertion_handler" COPYONLY) +# We generate the modulemap file so that we can include __config_site in it. For now, we don't know how to +# make __config_site modular when per-target runtime directories are used. +if (NOT LLVM_ENABLE_PER_TARGET_RUNTIME_DIR) + set(LIBCXX_CONFIG_SITE_MODULE_ENTRY "header \"__config_site\"") +endif() +configure_file("module.modulemap.in" "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap" @ONLY) + set(_all_includes "${LIBCXX_GENERATED_INCLUDE_TARGET_DIR}/__config_site" - "${LIBCXX_GENERATED_INCLUDE_DIR}/__assertion_handler") + "${LIBCXX_GENERATED_INCLUDE_DIR}/__assertion_handler" + "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap") foreach(f ${files}) set(src "${CMAKE_CURRENT_SOURCE_DIR}/${f}") set(dst "${LIBCXX_GENERATED_INCLUDE_DIR}/${f}") @@ -1726,6 +1733,12 @@ if (LIBCXX_INSTALL_HEADERS) PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ COMPONENT cxx-headers) + # Install the generated modulemap file to the generic include dir. + install(FILES "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap" + DESTINATION "${LIBCXX_INSTALL_INCLUDE_DIR}" + PERMISSIONS OWNER_READ OWNER_WRITE GROUP_READ WORLD_READ + COMPONENT cxx-headers) + if (NOT CMAKE_CONFIGURATION_TYPES) add_custom_target(install-cxx-headers DEPENDS cxx-headers diff --git a/libcxx/include/module.modulemap b/libcxx/include/module.modulemap.in similarity index 99% rename from libcxx/include/module.modulemap rename to libcxx/include/module.modulemap.in index 353d9651093ef..af928a63f2315 100644 --- a/libcxx/include/module.modulemap +++ b/libcxx/include/module.modulemap.in @@ -1,6 +1,7 @@ // This module contains headers related to the configuration of the library. These headers // are free of any dependency on the rest of libc++. module std_config [system] { + @LIBCXX_CONFIG_SITE_MODULE_ENTRY@ // generated via CMake textual header "__config" textual header "__configuration/abi.h" textual header "__configuration/availability.h" diff --git a/libcxx/test/libcxx/Wnon_modular_include_in_module.compile.pass.cpp b/libcxx/test/libcxx/Wnon_modular_include_in_module.compile.pass.cpp new file mode 100644 index 0000000000000..1946d24a36109 --- /dev/null +++ b/libcxx/test/libcxx/Wnon_modular_include_in_module.compile.pass.cpp @@ -0,0 +1,20 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// REQUIRES: target={{.*}}-apple-{{.*}} + +// This test ensures that libc++ supports being compiled with modules enabled and with +// -Wnon-modular-include-in-module. This effectively checks that we don't include any +// non-modular header from the library. +// +// Since most underlying platforms are not modularized properly, this test currently only +// works on Apple platforms. + +// ADDITIONAL_COMPILE_FLAGS: -Wnon-modular-include-in-module -Wsystem-headers-in-module=std -fmodules -fcxx-modules + +#include diff --git a/libcxx/test/libcxx/lint/lint_headers.sh.py b/libcxx/test/libcxx/lint/lint_headers.sh.py index c5e582cb0f7cb..ab237c968da7e 100644 --- a/libcxx/test/libcxx/lint/lint_headers.sh.py +++ b/libcxx/test/libcxx/lint/lint_headers.sh.py @@ -11,7 +11,7 @@ def exclude_from_consideration(path): return ( path.endswith(".txt") - or path.endswith(".modulemap") + or path.endswith(".modulemap.in") or os.path.basename(path) == "__config" or os.path.basename(path) == "__config_site.in" or os.path.basename(path) == "libcxx.imp" diff --git a/libcxx/utils/libcxx/header_information.py b/libcxx/utils/libcxx/header_information.py index 9811b42d510ca..a505d37b65b81 100644 --- a/libcxx/utils/libcxx/header_information.py +++ b/libcxx/utils/libcxx/header_information.py @@ -15,7 +15,7 @@ def _is_header_file(file): """Returns whether the given file is a header file, i.e. not a directory or the modulemap file.""" return not file.is_dir() and not file.name in [ - "module.modulemap", + "module.modulemap.in", "CMakeLists.txt", "libcxx.imp", "__config_site.in", From 48a0fd266f27775e234148e3a13d6859f072ada9 Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Tue, 8 Apr 2025 17:12:40 -0400 Subject: [PATCH 2/4] Fix --- libcxx/include/CMakeLists.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt index f61b6ac6ad94f..6433176b75d8f 100644 --- a/libcxx/include/CMakeLists.txt +++ b/libcxx/include/CMakeLists.txt @@ -1669,7 +1669,9 @@ configure_file("${LIBCXX_ASSERTION_HANDLER_FILE}" "${LIBCXX_GENERATED_INCLUDE_DI # We generate the modulemap file so that we can include __config_site in it. For now, we don't know how to # make __config_site modular when per-target runtime directories are used. if (NOT LLVM_ENABLE_PER_TARGET_RUNTIME_DIR) - set(LIBCXX_CONFIG_SITE_MODULE_ENTRY "header \"__config_site\"") + set(LIBCXX_CONFIG_SITE_MODULE_ENTRY + "header \"__config_site\" + export *") endif() configure_file("module.modulemap.in" "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap" @ONLY) From 36b1a9ac8659cffaa50639051e4e5ae280a96ead Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Wed, 9 Apr 2025 16:03:04 -0400 Subject: [PATCH 3/4] Better fix --- libcxx/include/CMakeLists.txt | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt index 6433176b75d8f..f1bdf684a8549 100644 --- a/libcxx/include/CMakeLists.txt +++ b/libcxx/include/CMakeLists.txt @@ -1669,9 +1669,7 @@ configure_file("${LIBCXX_ASSERTION_HANDLER_FILE}" "${LIBCXX_GENERATED_INCLUDE_DI # We generate the modulemap file so that we can include __config_site in it. For now, we don't know how to # make __config_site modular when per-target runtime directories are used. if (NOT LLVM_ENABLE_PER_TARGET_RUNTIME_DIR) - set(LIBCXX_CONFIG_SITE_MODULE_ENTRY - "header \"__config_site\" - export *") + set(LIBCXX_CONFIG_SITE_MODULE_ENTRY "textual header \"__config_site\"") endif() configure_file("module.modulemap.in" "${LIBCXX_GENERATED_INCLUDE_DIR}/module.modulemap" @ONLY) From be86a01b370bea6e655c654e4513d264b261bbd5 Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Wed, 9 Apr 2025 21:22:02 -0400 Subject: [PATCH 4/4] Fix test --- libcxx/test/libcxx/headers_in_modulemap.sh.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcxx/test/libcxx/headers_in_modulemap.sh.py b/libcxx/test/libcxx/headers_in_modulemap.sh.py index 51b14377ba89b..d4a18a20c8926 100644 --- a/libcxx/test/libcxx/headers_in_modulemap.sh.py +++ b/libcxx/test/libcxx/headers_in_modulemap.sh.py @@ -4,7 +4,7 @@ sys.path.append(sys.argv[1]) from libcxx.header_information import all_headers, libcxx_include -with open(libcxx_include / "module.modulemap") as f: +with open(libcxx_include / "module.modulemap.in") as f: modulemap = f.read() isHeaderMissing = False