[clang] [clang-tools-extra] [llvm] [clang][modules] stdarg.h and stddef.h shouldn't directly declare anything (PR #90676)

Ian Anderson via cfe-commits cfe-commits at lists.llvm.org
Thu May 2 16:07:40 PDT 2024


https://github.com/ian-twilightcoder updated https://github.com/llvm/llvm-project/pull/90676

>From 12964a3632d81d500b76c1a8a5c83a6367c66e00 Mon Sep 17 00:00:00 2001
From: Ian Anderson <iana at apple.com>
Date: Tue, 30 Apr 2024 15:16:38 -0700
Subject: [PATCH] [clang][modules] stdarg.h and stddef.h shouldn't directly
 declare anything

stdarg.h and especially stddef.h are textual and so everything they declare gets precompiled into all of their clients' pcm files. They shouldn't directly declare anything though, their purpose is to select what submodules get imported, and not to add duplicate declarations to all of their clients. Make it so that they always ignore their header guards, even without modules, and declare them in separate header files so that they only go into the stdarg/stddef pcms. Still declare them in case clients rely on them.
---
 .../find-all-symbols/STLPostfixHeaderMap.cpp  |  2 ++
 .../clangd/index/CanonicalIncludes.cpp        |  2 ++
 clang/lib/Headers/CMakeLists.txt              |  2 ++
 clang/lib/Headers/__stdarg_header_macro.h     | 12 ++++++++
 clang/lib/Headers/__stddef_header_macro.h     | 12 ++++++++
 clang/lib/Headers/module.modulemap            |  9 ++++++
 clang/lib/Headers/stdarg.h                    | 28 ++++-------------
 clang/lib/Headers/stddef.h                    | 30 ++++---------------
 .../gn/secondary/clang/lib/Headers/BUILD.gn   |  2 ++
 9 files changed, 53 insertions(+), 46 deletions(-)
 create mode 100644 clang/lib/Headers/__stdarg_header_macro.h
 create mode 100644 clang/lib/Headers/__stddef_header_macro.h

diff --git a/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp b/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
index df77bf7ea46da3..469323f0ee9d7f 100644
--- a/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
+++ b/clang-tools-extra/clang-include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
@@ -15,9 +15,11 @@ const HeaderMapCollector::RegexHeaderMap *getSTLPostfixHeaderMap() {
   static const HeaderMapCollector::RegexHeaderMap STLPostfixHeaderMap = {
       {"include/__stdarg___gnuc_va_list.h$", "<cstdarg>"},
       {"include/__stdarg___va_copy.h$", "<cstdarg>"},
+      {"include/__stdarg_header_macro.h$", "<cstdarg>"},
       {"include/__stdarg_va_arg.h$", "<cstdarg>"},
       {"include/__stdarg_va_copy.h$", "<cstdarg>"},
       {"include/__stdarg_va_list.h$", "<cstdarg>"},
+      {"include/__stddef_header_macro.h$", "<cstddef>"},
       {"include/__stddef_max_align_t.h$", "<cstddef>"},
       {"include/__stddef_null.h$", "<cstddef>"},
       {"include/__stddef_nullptr_t.h$", "<cstddef>"},
diff --git a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
index 42eeba36a80e43..785ec4086ea760 100644
--- a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -18,9 +18,11 @@ namespace {
 const std::pair<llvm::StringRef, llvm::StringRef> IncludeMappings[] = {
     {"include/__stdarg___gnuc_va_list.h", "<cstdarg>"},
     {"include/__stdarg___va_copy.h", "<cstdarg>"},
+    {"include/__stdarg_header_macro.h", "<cstdarg>"},
     {"include/__stdarg_va_arg.h", "<cstdarg>"},
     {"include/__stdarg_va_copy.h", "<cstdarg>"},
     {"include/__stdarg_va_list.h", "<cstdarg>"},
+    {"include/__stddef_header_macro.h", "<cstddef>"},
     {"include/__stddef_max_align_t.h", "<cstddef>"},
     {"include/__stddef_null.h", "<cstddef>"},
     {"include/__stddef_nullptr_t.h", "<cstddef>"},
diff --git a/clang/lib/Headers/CMakeLists.txt b/clang/lib/Headers/CMakeLists.txt
index 3416811e39de27..5f02c71f6ca51a 100644
--- a/clang/lib/Headers/CMakeLists.txt
+++ b/clang/lib/Headers/CMakeLists.txt
@@ -12,6 +12,7 @@ set(core_files
   stdarg.h
   __stdarg___gnuc_va_list.h
   __stdarg___va_copy.h
+  __stdarg_header_macro.h
   __stdarg_va_arg.h
   __stdarg_va_copy.h
   __stdarg_va_list.h
@@ -19,6 +20,7 @@ set(core_files
   stdbool.h
   stdckdint.h
   stddef.h
+  __stddef_header_macro.h
   __stddef_max_align_t.h
   __stddef_null.h
   __stddef_nullptr_t.h
diff --git a/clang/lib/Headers/__stdarg_header_macro.h b/clang/lib/Headers/__stdarg_header_macro.h
new file mode 100644
index 00000000000000..beb92ee0252679
--- /dev/null
+++ b/clang/lib/Headers/__stdarg_header_macro.h
@@ -0,0 +1,12 @@
+/*===---- __stdarg_header_macro.h ------------------------------------------===
+ *
+ * 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
+ *
+ *===-----------------------------------------------------------------------===
+ */
+
+#ifndef __STDARG_H
+#define __STDARG_H
+#endif
diff --git a/clang/lib/Headers/__stddef_header_macro.h b/clang/lib/Headers/__stddef_header_macro.h
new file mode 100644
index 00000000000000..db5fb3c0abc1bf
--- /dev/null
+++ b/clang/lib/Headers/__stddef_header_macro.h
@@ -0,0 +1,12 @@
+/*===---- __stddef_header_macro.h ------------------------------------------===
+ *
+ * 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
+ *
+ *===-----------------------------------------------------------------------===
+ */
+
+#ifndef __STDDEF_H
+#define __STDDEF_H
+#endif
diff --git a/clang/lib/Headers/module.modulemap b/clang/lib/Headers/module.modulemap
index 8741968fa7f364..4abfd1d98a6351 100644
--- a/clang/lib/Headers/module.modulemap
+++ b/clang/lib/Headers/module.modulemap
@@ -203,6 +203,11 @@ module _Builtin_stdarg [system] {
     export *
   }
 
+  explicit module header_macro {
+    header "__stdarg_header_macro.h"
+    export *
+  }
+
   explicit module va_arg {
     header "__stdarg_va_arg.h"
     export *
@@ -232,6 +237,10 @@ module _Builtin_stdbool [system] {
 module _Builtin_stddef [system] {
   textual header "stddef.h"
 
+  explicit module header_macro {
+    header "__stddef_header_macro.h"
+    export *
+  }
   // __stddef_max_align_t.h is always in this module, even if
   // -fbuiltin-headers-in-system-modules is passed.
   explicit module max_align_t {
diff --git a/clang/lib/Headers/stdarg.h b/clang/lib/Headers/stdarg.h
index 6e7bd604b2df41..8292ab907becf1 100644
--- a/clang/lib/Headers/stdarg.h
+++ b/clang/lib/Headers/stdarg.h
@@ -14,27 +14,13 @@
  * need to use some of its interfaces. Otherwise this header provides all of
  * the expected interfaces.
  *
- * When clang modules are enabled, this header is a textual header. It ignores
- * its header guard so that multiple submodules can export its interfaces.
- * Take module SM with submodules A and B, whose headers both include stdarg.h
- * When SM.A builds, __STDARG_H will be defined. When SM.B builds, the
- * definition from SM.A will leak when building without local submodule
- * visibility. stdarg.h wouldn't include any of its implementation headers, and
- * SM.B wouldn't import any of the stdarg modules, and SM.B's `export *`
- * wouldn't export any stdarg interfaces as expected. However, since stdarg.h
- * ignores its header guard when building with modules, it all works as
- * expected.
- *
- * When clang modules are not enabled, the header guards can function in the
- * normal simple fashion.
+ * When clang modules are enabled, this header is a textual header to support
+ * the multiple include behavior. As such, it doesn't directly declare anything
+ * so that it doesn't add duplicate declarations to all of its includers'
+ * modules.
  */
-#if !defined(__STDARG_H) || __has_feature(modules) ||                          \
-    defined(__need___va_list) || defined(__need_va_list) ||                    \
-    defined(__need_va_arg) || defined(__need___va_copy) ||                     \
-    defined(__need_va_copy)
-
 #if defined(__MVS__) && __has_include_next(<stdarg.h>)
-#define __STDARG_H
+#include <__stdarg_header_macro.h>
 #undef __need___va_list
 #undef __need_va_list
 #undef __need_va_arg
@@ -46,7 +32,7 @@
 #if !defined(__need___va_list) && !defined(__need_va_list) &&                  \
     !defined(__need_va_arg) && !defined(__need___va_copy) &&                   \
     !defined(__need_va_copy)
-#define __STDARG_H
+#include <__stdarg_header_macro.h>
 #define __need___va_list
 #define __need_va_list
 #define __need_va_arg
@@ -87,5 +73,3 @@
 #endif /* defined(__need_va_copy) */
 
 #endif /* __MVS__ */
-
-#endif
diff --git a/clang/lib/Headers/stddef.h b/clang/lib/Headers/stddef.h
index 9ccc0a68fbff33..8985c526e8fc53 100644
--- a/clang/lib/Headers/stddef.h
+++ b/clang/lib/Headers/stddef.h
@@ -14,30 +14,13 @@
  * need to use some of its interfaces. Otherwise this header provides all of
  * the expected interfaces.
  *
- * When clang modules are enabled, this header is a textual header. It ignores
- * its header guard so that multiple submodules can export its interfaces.
- * Take module SM with submodules A and B, whose headers both include stddef.h
- * When SM.A builds, __STDDEF_H will be defined. When SM.B builds, the
- * definition from SM.A will leak when building without local submodule
- * visibility. stddef.h wouldn't include any of its implementation headers, and
- * SM.B wouldn't import any of the stddef modules, and SM.B's `export *`
- * wouldn't export any stddef interfaces as expected. However, since stddef.h
- * ignores its header guard when building with modules, it all works as
- * expected.
- *
- * When clang modules are not enabled, the header guards can function in the
- * normal simple fashion.
+ * When clang modules are enabled, this header is a textual header to support
+ * the multiple include behavior. As such, it doesn't directly declare anything
+ * so that it doesn't add duplicate declarations to all of its includers'
+ * modules.
  */
-#if !defined(__STDDEF_H) || __has_feature(modules) ||                          \
-    (defined(__STDC_WANT_LIB_EXT1__) && __STDC_WANT_LIB_EXT1__ >= 1) ||        \
-    defined(__need_ptrdiff_t) || defined(__need_size_t) ||                     \
-    defined(__need_rsize_t) || defined(__need_wchar_t) ||                      \
-    defined(__need_NULL) || defined(__need_nullptr_t) ||                       \
-    defined(__need_unreachable) || defined(__need_max_align_t) ||              \
-    defined(__need_offsetof) || defined(__need_wint_t)
-
 #if defined(__MVS__) && __has_include_next(<stddef.h>)
-#define __STDDEF_H
+#include <__stddef_header_macro.h>
 #undef __need_ptrdiff_t
 #undef __need_size_t
 #undef __need_rsize_t
@@ -57,7 +40,7 @@
     !defined(__need_NULL) && !defined(__need_nullptr_t) &&                     \
     !defined(__need_unreachable) && !defined(__need_max_align_t) &&            \
     !defined(__need_offsetof) && !defined(__need_wint_t)
-#define __STDDEF_H
+#include <__stddef_header_macro.h>
 #define __need_ptrdiff_t
 #define __need_size_t
 /* ISO9899:2011 7.20 (C11 Annex K): Define rsize_t if __STDC_WANT_LIB_EXT1__ is
@@ -137,4 +120,3 @@ __WINT_TYPE__ directly; accommodate both by requiring __need_wint_t */
 #endif /* __need_wint_t */
 
 #endif /* __MVS__ */
-#endif
diff --git a/llvm/utils/gn/secondary/clang/lib/Headers/BUILD.gn b/llvm/utils/gn/secondary/clang/lib/Headers/BUILD.gn
index 971ceb3185ff83..0a7cc3854056c0 100644
--- a/llvm/utils/gn/secondary/clang/lib/Headers/BUILD.gn
+++ b/llvm/utils/gn/secondary/clang/lib/Headers/BUILD.gn
@@ -105,9 +105,11 @@ copy("Headers") {
     "__clang_hip_stdlib.h",
     "__stdarg___gnuc_va_list.h",
     "__stdarg___va_copy.h",
+    "__stdarg_header_macro.h",
     "__stdarg_va_arg.h",
     "__stdarg_va_copy.h",
     "__stdarg_va_list.h",
+    "__stddef_header_macro.h",
     "__stddef_max_align_t.h",
     "__stddef_null.h",
     "__stddef_nullptr_t.h",



More information about the cfe-commits mailing list