[Headers] Remove musl-related comment about NULL

[aheejin]

Summary

This removes a comment added in D159312, which warned people to not
re-add a whitespace in the ((void*)0)) expression. After discussions
happened in D159312, it doesn't seem like a permanent solution.

While I'd like to keep the whitespace removed for now, given that at
least it can be a band-aid to some users who use musl and clang's
stddef.h at the same time, it seems the usage of them together is not
something that's officially supported, and I should not be implying this
should be the permanent solution by saying so in the comments.

[ributzka]

LGTM

[dschuff]

Suggested edit to the commit description:
"use musl and stddef.h at the same time" -> "use musl and clang's stddef.h at the same time"

[aheejin]

In D159383#4635703, @dschuff wrote:

Suggested edit to the commit description:
"use musl and stddef.h at the same time" -> "use musl and clang's stddef.h at the same time"

Done (Both the CL description and the commit message, which will be shown after the actual commit)

[aaron.ballman]

As far as the change goes, this is fine (it keeps the status quo we previously had), but I still wonder if we should be trying to detect that the user is working with musl and do an include_next so our stddef.h header never gets involved in musl's standard library headers except as a pass-through. If @dalias thinks that would be beneficial to musl, it may be worth doing in a follow-up.

[dalias]

musl targets already have the libc header path before the compiler header path in a BSD-like configuration (the BSDs also have this order on GCC and I would assume on clang as well), so if the compiler-provided stddef.h is getting seen at all, there's either a bug in clang's path logic or (more likely) user error, like trying to use musl with a non-musl target tuple, no "system headers" path, and simple -I to point at the musl headers.

[dschuff]

The 2 common WebAssembly toolchain variants (Emscripten and wasi-sdk) use libcs that are derived from musl (a subset along with additions specific to their environments); they share the system-include configuration in WebAssembly::AddClangSystemIncludeArgs() in clang/lib/Driver/Toolchains/WebAssembly.cpp, which adds the ResourceDir before the Sysroot (and that I believe results in this include order). So we should maybe consider fixing that (something @aheejin and I should discuss with @sunfish and @sbc100 and the rest of the Emscripten and/or wasi folks).

[dschuff]

This comment was removed by dschuff.

[dschuff]

Although having said all that, I guess a question for @aaron.ballman or other clang header experts: It does seem that many std C headers in clang are designed to handle this kind of case using include_next (e.g. stdint.h and stdatomic.h) but not all of them (e.g. stddef.h or stdbool.h). So maybe we should try to make them consistent anyway.

[dalias]

To be clear, musl does not support this regardless of whether #include_next is being used "right" or not. The only supported thing is not mixing headers at all.

If Emscripten and wasi are using musl-based headers, it probably makes sense for these targets to do the same thing with include order as for musl targets. The practice of mixing responsibility for system headers between libc and the compiler is a very glibc+gcc specific legacy thing. Even on gcc, the BSD targets don't do this. Other targets might just because the gcc folks had a thing about "fixing" proprietary unices' headers, but the standard open source non-GNU thing to do is to leave the headers entirely as the libc's responsibility.

[sunfish]

I believe wasi-sdk doesn't have this issue, because it has modifications to all headers which define NULL to always use the NULL defined in stddef.h, and it doesn't ship musl's stddef.h.

clang/lib/Headers/__stddef_null.h

 #undef NULL
 #ifdef __cplusplus
 #if !defined(__MINGW32__) && !defined(_MSC_VER)
 #define NULL __null
 #else
 #define NULL 0
 #endif
 #else
-// Don't add any whitespaces in ((void*)0) below!
-// musl (https://www.musl-libc.org/) redefines `NULL` as such and redefinition
-// with a different expression, even in terms of a single whitespace, causes a
-// warning.
 #define NULL ((void*)0)
 #endif