Make a new header to define NULL so that the OS can use it independent of stddef.h, i.e. in POSIX headers that are required to define NULL but not the rest of stddef.h.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
clang/lib/Headers/__stddef_null.h | ||
---|---|---|
16 | arc lint wanted to remove all of the indentation that was in stddef.h. |
Add a module for stddef_null.h and give it the same special treatment as stddef_max_align_t.h.
Rebase, update diagnostic to include the full module name instead of just the top level.
llvm/test/DebugInfo/Generic/assignment-tracking/sroa/unspecified-var-size.ll | ||
---|---|---|
37 | Adding this line is the only reason I changed this file. I'm not familiar at all with how these tests work, so I don't really know if it's necessary. The test passes with and without these changes. |
It's covered pretty well by stddefneeds.cpp already. I can add a new one if you want, but it'd pretty much be a copy/paste
I was thinking of one similar to that but includes the new header and *not* stddef.h to demonstrate that this works entirely standalone. WDYT?
llvm/test/DebugInfo/Generic/assignment-tracking/sroa/unspecified-var-size.ll | ||
---|---|---|
37 | Don't think there's any reason/need to - what motivated changing this file? |
llvm/test/DebugInfo/Generic/assignment-tracking/sroa/unspecified-var-size.ll | ||
---|---|---|
37 | I found it when I was checking for places that handled __stddef_max_align_t.h specially. I'm not sure if that's in here to check _ZTS11max_align_t under it, or if it's just there because stddef.h includes it. __stddef_null.h doesn't define any types like that, so maybe it's fine to just revert this file? |
llvm/test/DebugInfo/Generic/assignment-tracking/sroa/unspecified-var-size.ll | ||
---|---|---|
37 | Yeah, I think it's reasonable to revert the changes to this file. |
llvm/test/DebugInfo/Generic/assignment-tracking/sroa/unspecified-var-size.ll | ||
---|---|---|
37 | Yeah, that's just some incidental debug info (because clang doesn't track use of using decls, we emit them rather unconditionally into DWARF, and it's often easier to create/manitain DWARF testing by compiling real code rather than hand crafting the IR metadata) - no need to update it for this case, I think. |
Revert the unnecessary changes to unspecified-var-size.ll
llvm/test/DebugInfo/Generic/assignment-tracking/sroa/unspecified-var-size.ll | ||
---|---|---|
37 | 👍 |
Wait a second... if an OS wants only NULL, we already have a supported way of achieving that, which is compatible with GCC and glibc and other POSIX compilers -- define __need_NULL before including the header.
We shouldn't be providing internal headers with __mangled_names for OSes to include, those should be implementation details.
You can't do that with modules though, #define __need_NULL #include <stddef.h> doesn't work.
Also, from the patch summary: i.e. in POSIX headers that are required to define NULL but not the rest of stddef.h. -- I don't think __need_NULL works for this because it gets you NULL but it also gets you the rest of the stddef.h header, right?
Once you set one of the __need macros, then stddef.h will only give you those definitions. Or I'm interpreting the file gravely wrong... it's kind of a weird header and its customizations don't really work with modules either.
More concretely, on Apple platforms there's <sys/_types/_null.h>, <sys/_types/_ptrdiff.h>, etc. In textual mode you could have this.
// _null.h #define __need_NULL #include <stddef.h> // _ptrdiff.h #define __need_ptrdiff_t #include <stddef.h>
But that doesn't work with modules. stddef.h is built once according to whatever macros were passed on the command line. And there's no way to set the macros on the command line such that each OS header gets just what it needs. Hence the need to split NULL, and really all of stddef.h but I'm going to do that in a future change, into constituent headers.
Our builtin header stddef.h shouldn't be built as a module. It fundamentally needs to be treated as a textual header, because it consumes macros defined by the includer. The module map that we provide with our builtin headers does not cover stddef.h for that reason.
That said... the __has_feature(modules) checks throughout stddef.h look pretty wrong to me. They're probably working around the infelicities of the -fno-modules-local-submodule-visibility mode; maybe we can disable them when not in that mode?
It ends up being covered by Darwin.C.stddef on Apple platforms (due to some builtin shenanigans elsewhere in clang). Maybe we could make stddef.h be a textual header in a _Builtin_stddef module in clang and that would do the trick instead of splitting it into a bunch of different files.
This seems to work in initial testing. I'll probably drop this diff and make a new one to make stddef.h be a textual header in a new _Builtin_stddef module in clang when I do some more testing then.
arc lint wanted to remove all of the indentation that was in stddef.h.