[PATCH] D157757: [Headers] Replace __need_STDDEF_H_misc with specific __need_ macros

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Headers/stddef.h:118-122
+#ifdef __cplusplus
+namespace std {
+typedef decltype(nullptr) nullptr_t;
+}
+using ::std::nullptr_t;
----------------
iana wrote:
> aaron.ballman wrote:
> > iana wrote:
> > > ldionne wrote:
> > > > iana wrote:
> > > > > aaron.ballman wrote:
> > > > > > Related:
> > > > > > 
> > > > > > https://github.com/llvm/llvm-project/issues/37564
> > > > > > https://cplusplus.github.io/LWG/issue3484
> > > > > > 
> > > > > > CC @ldionne
> > > > > I don't _think_ this change actually changes the way nullptr_t gets defined in C++, does it?
> > > > I think we absolutely don't want to touch `std::nullptr_t` from this header. It's libc++'s responsibility to define that, and in fact we define it in `std::__1`, so this is even an ABI break (or I guess it would be a compiler error, not sure).
> > > I'm really not touching it though. All I did is move it from `__need_NULL` to `__need_nullptr_t`.
> > > 
> > > The old behavior is that `std::nullptr_t` would only be touched if (no `__need_` macros were set or if `__need_NULL` was set), and (_MSC_EXTENSIONS and _NATIVE_NULLPTR_SUPPORTED are defined).
> > > 
> > > The new behavior is that `std::nullptr_t` will only be touched if ((no `__need_` macros are set) and (_MSC_EXTENSIONS and _NATIVE_NULLPTR_SUPPORTED are defined)) or (the new `__need_nullptr_t` macro is set)
> > > 
> > > So the only change is that C++ code that previously set `__need_NULL` will no longer get `std::nullptr_t`. @efriedma felt like that was a fine change.
> > Does libc++ provide the symbols for a freestanding compilation?
> > 
> > I was pointing out those links specifically because the C++ standard currently says that stddef.h (the C standard library header) needs to provide a definition of `std::nullptr_t`, but that LWG thinks that's perhaps not the right way to do that and may be removing that requirement.
> It is weird the standard puts that in stddef.h and not cstddef. I think libc++ could provide that in their stddef.h anyway, but the intent in this review is to not rock the boat and only do the minimal change discussed above.
Yeah, this discussion is to figure out whether we have an existing bug we need to address and if so, where to address it (libc++, clang, or the C++ standard). I don't think your changes are exacerbating anything, more just that they've potentially pointed out something related.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157757/new/

https://reviews.llvm.org/D157757