aaron.ballman added inline comments.
================
Comment at: clang/lib/Headers/stddef.h:1
/*===---- stddef.h - Basic type definitions --------------------------------===
*
----------------
ldionne wrote:
> Making a thread out of this:
>
> > The relationship between clang's stddef.h and the C Standard Library
> > stddef.h is that there is no relationship. clang's header doesn't
> > #include_next, and it is in the search path before the OS's cstdlib.
>
> So in that case what is the purpose of the SDK/system providing a
> `<stddef.h>` header? They basically all provide one and it's never used?
>
The compiler provides `<stddef.h>` for the same reason it provides `<limits.h>`
and others: the compiler is responsible for defining these interfaces because
it's the only thing that knows the correct definitions it expects. The system
might know some of it, but for example, `size_t` relates to the maximum size of
an object, which is something only the compiler knows the answer to.
================
Comment at: clang/lib/Headers/stddef.h:118-122
+#ifdef __cplusplus
+namespace std {
+typedef decltype(nullptr) nullptr_t;
+}
+using ::std::nullptr_t;
----------------
ldionne wrote:
> iana wrote:
> > aaron.ballman wrote:
> > > 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.
> > 👍
> > Does libc++ provide the symbols for a freestanding compilation?
>
> I don't think we do. We basically don't support `-ffreestanding` right now
> (we support embedded and funky platforms via other mechanisms).
>
> But regardless, `<stddef.h>` should never define something in namespace
> `std`, that should be libc++'s responsibility IMO. What we could do here
> instead is just
>
> ```
> #ifdef __cplusplus
> typedef decltype(nullptr) nullptr_t;
> #else
> typedef typeof(nullptr) nullptr_t;
> #endif
> ```
>
> and then let libc++'s `<cstddef>` do
>
> ```
> _LIBCPP_BEGIN_NAMESPACE_STD
> using ::nullptr_t;
> _LIBCPP_END_NAMESPACE_STD
> ```
>
> If Clang's `<stddef.h>` did define `::nullptr_t`, we could likely remove
> libc++'s `<stddef.h>` and that might simplify things.
>> Does libc++ provide the symbols for a freestanding compilation?
> I don't think we do. We basically don't support -ffreestanding right now (we
> support embedded and funky platforms via other mechanisms).
Okay, that's what I thought as well. Thanks!
> But regardless, <stddef.h> should never define something in namespace std,
> that should be libc++'s responsibility IMO. What we could do here instead is
> just
Ah, so you're thinking stddef.h should provide the global nullptr_t and cstddef
should provide the std::nullptr_t. I was thinking stddef.h should not define
nullptr_t in C++ mode at all; it's a C header, not a C++ header. That led me to
thinking about what the behavior should be in C23 given that it supports
nullptr_t.
Were it not for the current requirement that stddef.h provide nullptr_t, I
think stddef.h should do:
```
typedef typeof(nullptr) nullptr_t;
```
in C23 mode and not do anything special for C++ at all. C's `nullptr_t` needs
to be ABI compatible with C++'s `nullptr_t`, so a C++ user including the C
header should not get any problems linking against a C++ library. However, this
would mean that C++ users cannot include stddef.h to get nullptr_t; they'd need
to include cstddef to be assured they'd get it. But because of the ABI
compatibility, perhaps the solution is to expose the above in both C and C++
modes from stddef.h, then libc++ can do the dance to import it into namespace
std?
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
