[clang][modules] stdarg.h and stddef.h shouldn't directly declare anything

[ian-twilightcoder]

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.

[llvmbot]

@llvm/pr-subscribers-clangd
@llvm/pr-subscribers-clang-tools-extra
@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-clang

Author: Ian Anderson (ian-twilightcoder)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/90676.diff

5 Files Affected:

• (added) clang/lib/Headers/__stdarg_macro.h (+12)
• (added) clang/lib/Headers/__stddef_macro.h (+12)
• (modified) clang/lib/Headers/module.modulemap (+9)
• (modified) clang/lib/Headers/stdarg.h (+5-19)
• (modified) clang/lib/Headers/stddef.h (+5-22)

diff --git a/clang/lib/Headers/__stdarg_macro.h b/clang/lib/Headers/__stdarg_macro.h
new file mode 100644
index 00000000000000..aa9c9f72f1c1ac
--- /dev/null
+++ b/clang/lib/Headers/__stdarg_macro.h
@@ -0,0 +1,12 @@
+/*===---- __stdarg_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_macro.h b/clang/lib/Headers/__stddef_macro.h
new file mode 100644
index 00000000000000..dac7b1b47cd80a
--- /dev/null
+++ b/clang/lib/Headers/__stddef_macro.h
@@ -0,0 +1,12 @@
+/*===---- __stddef_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..492d2aba8af969 100644
--- a/clang/lib/Headers/module.modulemap
+++ b/clang/lib/Headers/module.modulemap
@@ -203,6 +203,11 @@ module _Builtin_stdarg [system] {
     export *
   }
 
+  explicit module macro {
+    header "__stdarg_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 macro {
+    header "__stddef_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 94b066566f084e..753c187cb61841 100644
--- a/clang/lib/Headers/stdarg.h
+++ b/clang/lib/Headers/stdarg.h
@@ -14,29 +14,15 @@
  * 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(__need___va_list) && !defined(__need_va_list) &&                  \
     !defined(__need_va_arg) && !defined(__need___va_copy) &&                   \
     !defined(__need_va_copy)
-#define __STDARG_H
+#include <__stdarg_macro.h>
 #define __need___va_list
 #define __need_va_list
 #define __need_va_arg
diff --git a/clang/lib/Headers/stddef.h b/clang/lib/Headers/stddef.h
index e0ad7b8d17aff9..8ab3bfbd8a2eba 100644
--- a/clang/lib/Headers/stddef.h
+++ b/clang/lib/Headers/stddef.h
@@ -14,34 +14,17 @@
  * 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(__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)
-#define __STDDEF_H
+#include <__stddef_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

[Bigcheese]

I think I like this as a solution, although I wonder if __stdarg_guard_macro.h would be a better name. The name now somewhat implies that it contains the macros that stddef and stdarg define.

[ian-twilightcoder]

I think I like this as a solution, although I wonder if __stdarg_guard_macro.h would be a better name. The name now somewhat implies that it contains the macros that stddef and stdarg define.

But the macros don't actually guard anything anymore. _macro isn't the best name in the world though, maybe _header_macro.h is ok even though they aren't traditional "header macros"?

[Bigcheese]

lgtm