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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
