[clang] [clang][headers] Including stddef.h always redefines NULL (PR #99727)

[Richard Smith via cfe-commits]

================
@@ -49,7 +48,24 @@
 #define __need_rsize_t
 #endif
 #define __need_wchar_t
+#if !defined(__STDDEF_H) || __has_feature(modules)
+/*
+ * __stddef_null.h is special when building without modules: if __need_NULL is
+ * set, then it will unconditionally redefine NULL. To avoid stepping on client
+ * definitions of NULL, __need_NULL should only be set the first time this
+ * header is included, that is when __STDDEF_H is not defined. However, when
+ * building with modules, this header is a textual header and needs to
+ * unconditionally include __stdef_null.h to support multiple submodules
+ * exporting _Builtin_stddef.null. 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 __stddef_null.h, and
+ * SM.B wouldn't import _Builtin_stddef.null, and SM.B's `export *` wouldn't
+ * export NULL as expected. When building with modules, always include
+ * __stddef_null.h so that everything works as expected.
+ */
 #define __need_NULL
+#endif
----------------
zygoloid wrote:

As a general principle, it seems to me that getting C and C++ semantics right should be the primary concern here, and we shouldn't regress correctness in C or C++ to improve the behavior of Swift interop. Though as it happens, in this case the modules behavior does seem like a reasonable choice for C and C++ -- and really the issue is just that we have a divergence in behavior between modules and non-modules compilations.

I wonder if we could change our behavior for `__need_*` in non-modules builds to match the modules behavior: specifically that you only get each piece once, the first time you ask for it, so if you ask for just `NULL` then ask for everything, you don't get `NULL` the second time. If we think that's good enough for modules, then it seems like it should be good enough in general, and means there's less that can go wrong when turning on / off modules.

https://github.com/llvm/llvm-project/pull/99727