[libcxx-commits] [libcxx] [libc++] Make __config_site modular (PR #134699)

Konstantin Varlamov via libcxx-commits libcxx-commits at lists.llvm.org
Thu Apr 17 18:48:44 PDT 2025 

https://github.com/var-const updated https://github.com/llvm/llvm-project/pull/134699

>From a232a4c3b23687cd937d561ddc7eedc1602169df Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 7 Apr 2025 13:59:03 -0400
Subject: [PATCH 1/5] [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 <vector>
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 ccfb0f6e6967c8d691f0b0efa44c966b11569634 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Tue, 8 Apr 2025 17:12:40 -0400
Subject: [PATCH 2/5] 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 a9c6d5f791083f5c4d8d40d0c50d90fb9a48e188 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Wed, 9 Apr 2025 16:03:04 -0400
Subject: [PATCH 3/5] 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 59ca9c35bb078907f297f14834e7af89de50c9ef Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Wed, 9 Apr 2025 21:22:02 -0400
Subject: [PATCH 4/5] 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

>From e21b5858ecbc61d2d33198b2e481647026cd73c4 Mon Sep 17 00:00:00 2001
From: Konstantin Varlamov <varconst at apple.com>
Date: Thu, 17 Apr 2025 18:48:23 -0700
Subject: [PATCH 5/5] Xfail the new test in C++03 mode (the modules build tries
 to include main headers, not old frozen headers)

---
 .../test/libcxx/Wnon_modular_include_in_module.compile.pass.cpp  | 1 +
 1 file changed, 1 insertion(+)

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
index 1946d24a36109..86807576ee070 100644
--- a/libcxx/test/libcxx/Wnon_modular_include_in_module.compile.pass.cpp
+++ b/libcxx/test/libcxx/Wnon_modular_include_in_module.compile.pass.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 // REQUIRES: target={{.*}}-apple-{{.*}}
+// XFAIL: FROZEN-CXX03-HEADERS-FIXME
 
 // 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

More information about the libcxx-commits mailing list