diff --git a/clang/lib/Headers/stdarg.h b/clang/lib/Headers/stdarg.h index 8292ab907becf..6203d7a600a23 100644 --- a/clang/lib/Headers/stdarg.h +++ b/clang/lib/Headers/stdarg.h @@ -20,19 +20,18 @@ * modules. */ #if defined(__MVS__) && __has_include_next() -#include <__stdarg_header_macro.h> #undef __need___va_list #undef __need_va_list #undef __need_va_arg #undef __need___va_copy #undef __need_va_copy +#include <__stdarg_header_macro.h> #include_next #else #if !defined(__need___va_list) && !defined(__need_va_list) && \ !defined(__need_va_arg) && !defined(__need___va_copy) && \ !defined(__need_va_copy) -#include <__stdarg_header_macro.h> #define __need___va_list #define __need_va_list #define __need_va_arg @@ -45,6 +44,7 @@ !defined(__STRICT_ANSI__) #define __need_va_copy #endif +#include <__stdarg_header_macro.h> #endif #ifdef __need___va_list diff --git a/clang/lib/Headers/stddef.h b/clang/lib/Headers/stddef.h index 8985c526e8fc5..99b275aebf5aa 100644 --- a/clang/lib/Headers/stddef.h +++ b/clang/lib/Headers/stddef.h @@ -20,7 +20,6 @@ * modules. */ #if defined(__MVS__) && __has_include_next() -#include <__stddef_header_macro.h> #undef __need_ptrdiff_t #undef __need_size_t #undef __need_rsize_t @@ -31,6 +30,7 @@ #undef __need_max_align_t #undef __need_offsetof #undef __need_wint_t +#include <__stddef_header_macro.h> #include_next #else @@ -40,7 +40,6 @@ !defined(__need_NULL) && !defined(__need_nullptr_t) && \ !defined(__need_unreachable) && !defined(__need_max_align_t) && \ !defined(__need_offsetof) && !defined(__need_wint_t) -#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 @@ -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 #if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 202311L) || \ defined(__cplusplus) #define __need_nullptr_t @@ -65,6 +81,7 @@ /* wint_t is provided by and not . It's here * for compatibility, but must be explicitly requested. Therefore * __need_wint_t is intentionally not defined here. */ +#include <__stddef_header_macro.h> #endif #if defined(__need_ptrdiff_t) diff --git a/clang/test/Headers/stddefneeds.cpp b/clang/test/Headers/stddefneeds.cpp index 0763bbdee13ae..0282e8afa600d 100644 --- a/clang/test/Headers/stddefneeds.cpp +++ b/clang/test/Headers/stddefneeds.cpp @@ -56,14 +56,21 @@ max_align_t m5; #undef NULL #define NULL 0 -// glibc (and other) headers then define __need_NULL and rely on stddef.h -// to redefine NULL to the correct value again. -#define __need_NULL +// Including stddef.h again shouldn't redefine NULL #include // gtk headers then use __attribute__((sentinel)), which doesn't work if NULL // is 0. -void f(const char* c, ...) __attribute__((sentinel)); +void f(const char* c, ...) __attribute__((sentinel)); // expected-note{{function has been explicitly marked sentinel here}} void g() { + f("", NULL); // expected-warning{{missing sentinel in function call}} +} + +// glibc (and other) headers then define __need_NULL and rely on stddef.h +// to redefine NULL to the correct value again. +#define __need_NULL +#include + +void h() { f("", NULL); // Shouldn't warn. } diff --git a/clang/test/Modules/stddef.cpp b/clang/test/Modules/stddef.cpp new file mode 100644 index 0000000000000..c53bfa3485194 --- /dev/null +++ b/clang/test/Modules/stddef.cpp @@ -0,0 +1,73 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/no-lsv -I%t %t/stddef.cpp -verify +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-local-submodule-visibility -fmodules-cache-path=%t/lsv -I%t %t/stddef.cpp -verify + +//--- stddef.cpp +#include + +void *pointer = NULL; +size_t size = 0; + +// When building with modules, a pcm is never re-imported, so re-including +// stddef.h will not re-import _Builtin_stddef.null to restore the definition of +// NULL, even though stddef.h will unconditionally include __stddef_null.h when +// building with modules. +#undef NULL +#include + +void *anotherPointer = NULL; // expected-error{{use of undeclared identifier 'NULL'}} + +// stddef.h needs to be a `textual` header to support clients doing things like +// this. +// +// #define __need_NULL +// #include +// +// As a textual header designed to be included multiple times, it can't directly +// declare anything, or those declarations would go into every module that +// included it. e.g. if stddef.h contained all of its declarations, and modules +// A and B included stddef.h, they would both have the declaration for size_t. +// That breaks Swift, which uses the module name as part of the type name, i.e. +// A.size_t and B.size_t are treated as completely different types in Swift and +// cannot be interchanged. To fix that, stddef.h (and stdarg.h) are split out +// into a separate file per __need macro that can be normal headers in explicit +// submodules. That runs into yet another wrinkle though. When modules build, +// declarations from previous submodules leak into subsequent ones when not +// using local submodule visibility. Consider if stddef.h did the normal thing. +// +// #ifndef __STDDEF_H +// #define __STDDEF_H +// // include all of the sub-headers +// #endif +// +// When SM builds without local submodule visibility, it will precompile a.h +// first. When it gets to b.h, the __STDDEF_H declaration from precompiling a.h +// will leak, and so when b.h includes stddef.h, it won't include any of its +// sub-headers, and SM.B will thus not import _Builtin_stddef or make any of its +// submodules visible. Precompiling b.h will be fine since it sees all of the +// declarations from a.h including stddef.h, but clients that only include b.h +// will not see any of the stddef.h types. stddef.h thus has to make sure to +// always include the necessary sub-headers, even if they've been included +// already. They all have their own header guards to allow this. +// __stddef_null.h is extra special, so this test makes sure to cover NULL plus +// one of the normal stddef.h types. + +//--- module.modulemap +module SM { + module A { + header "a.h" + export * + } + + module B { + header "b.h" + export * + } +} + +//--- a.h +#include + +//--- b.h +#include