================ @@ -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:
OK, I see: we want a plain include of `<stddef.h>` to define `NULL` only the first time. In non-modules builds, we get this by checking for `__STDDEF_H`, and in modules builds we get this because `__stddef_null.h` is a modular header. (And checking for `__STDDEF_H` isn't sufficient for a modules but non-LSV build because it might leak from a different submodule.) So it doesn't matter whether we define `__need_NULL` for a modules LSV build; it does the same thing either way. It looks like we would give different behavior in a modules build for something like: ``` #define __need_NULL #include <stddef.h> #undef NULL #include <stddef.h> ``` ... which normally results in `NULL` being defined, but with any flavor of modules, results in `NULL` *not* being defined -- because we build `__stddef_null.h` as a modular header, even though it's really not (it's supposed to redefine `NULL` each time it's included). But that's a pre-existing issue. In any case, I think I'm convinced this change is correct. Thanks for bearing with me :) https://github.com/llvm/llvm-project/pull/99727 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
