[libc++] stddef.h needs to #include_next for the new clang __need_ macros

[ian-twilightcoder]

clang added several __need_ macros to stddef.h that need to be added to the libc++ version as well. clang will also re-enter stddef.h to define rsize_t if __STDC_WANT_LIB_EXT1__ is set, do that in the libc++ one too.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Ian Anderson (ian-twilightcoder)

Changes

clang added several _need macros to stddef.h that need to be added to the libc++ version as well. clang will also re-enter stddef.h to define rsize_t if STDC_WANT_LIB_EXT1 is set, do that in the libc++ one too.

---

Full diff: https://github.com/llvm/llvm-project/pull/86252.diff

1 Files Affected:

• (modified) libcxx/include/stddef.h (+4-3)

diff --git a/libcxx/include/stddef.h b/libcxx/include/stddef.h
index 887776b150e49d..a3acf0ff10d1b6 100644
--- a/libcxx/include/stddef.h
+++ b/libcxx/include/stddef.h
@@ -7,8 +7,9 @@
 //
 //===----------------------------------------------------------------------===//
 
-#if defined(__need_ptrdiff_t) || defined(__need_size_t) || defined(__need_wchar_t) || defined(__need_NULL) ||          \
-    defined(__need_wint_t)
+#if defined(__need_ptrdiff_t) || defined(__need_size_t) || defined(__need_rsize_t) || defined(__need_wchar_t) ||       \
+    defined(__need_NULL) || defined(__need_nullptr_t) || defined(__need_unreachable) || defined(__need_max_align_t) || \
+    defined(__need_offsetof) || defined(__need_wint_t)
 
 #  if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #    pragma GCC system_header
@@ -16,7 +17,7 @@
 
 #  include_next <stddef.h>
 
-#elif !defined(_LIBCPP_STDDEF_H)
+#elif !defined(_LIBCPP_STDDEF_H) || (defined(__STDC_WANT_LIB_EXT1__) && __STDC_WANT_LIB_EXT1__ >= 1)
 #  define _LIBCPP_STDDEF_H
 
 /*

[ldionne]

These macros are obscure. I would, in fact, much rather remove any knowledge of __need_ macros from our headers.

[ian-twilightcoder]

These macros are obscure. I would, in fact, much rather remove any knowledge of __need_ macros from our headers.

I guess we could try removing the macros and the header guard and just always include_next. That feels like a riskier change for llvm 18 though.

[ian-twilightcoder]

These macros are obscure. I would, in fact, much rather remove any knowledge of __need_ macros from our headers.

I guess we could try removing the macros and the header guard and just always include_next. That feels like a riskier change for llvm 18 though.

Which is to say I'd rather do the safe thing right now and leave this PR the way it is, which is just to fixup the way this header already works rather than try to clean it up. At least for this release, if that's ok with you.

[ldionne]

I would actually suggest that we change the whole header to this:

// -*- C++ -*-
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

/*
    stddef.h synopsis

Macros:

    offsetof(type,member-designator)
    NULL

Types:

    ptrdiff_t
    size_t
    max_align_t // C++11
    nullptr_t

*/

#include <__config>

// Note: This include is outside of header guards because we sometimes get included multiple times
//       with different defines and the underlying <stddef.h> will know how to deal with that.
#include_next <stddef.h>

#ifndef _LIBCPP_STDDEF_H
#  define _LIBCPP_STDDEF_H

#  if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
#    pragma GCC system_header
#  endif

#  ifdef __cplusplus
typedef decltype(nullptr) nullptr_t;
#  endif

#endif // _LIBCPP_STDDEF_H

WDYT?

[ian-twilightcoder]

I don't think we can unconditionally declare nullptr_t. If e.g. __need_size_t is set, then nullptr_t shouldn't get declared. I'm not sure if that's for POSIX conformance or what, but it seems to be important when I talk to people implementing low level headers.

[ldionne]

I don't see what the problem would be though? We'd be defining it at most once due to the header guard, and nobody else anywhere should be defining nullptr_t anyways.

Either way, I can try doing that in a separate patch.

[ldionne]

Actually, I officially don't understand this patch anymore so I would argue we should go for the simpler approach suggested above unless there's a concrete reason why that wouldn't work.

[ian-twilightcoder]

The problem is that some headers need to declare just size_t, and it's a problem if anything else gets declared. The way they do that is by setting __need_size_t and including stddef.h. I think it's for POSIX reasons but I'm not 100% why it's so important for people to be able to only see pieces of stdarg and stddef.

[ian-twilightcoder]

@AaronBallman do you know why the __need_ macros are so important in the clang stddef.h?

[AaronBallman]

My understanding is that this is needed for interfacing with other CRTs (primarily Apple's?):
https://opensource.apple.com/source/Libc/Libc-825.26/include/stddef.h.auto.html
https://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20151005/140394.html

and was carried forward with newer functionality more recently:
https://reviews.llvm.org/D157757

[ldionne]

Thanks for digging this up. We're checking with some folks internally.

[ldionne]

The answer we're getting from the Darwin folks is that they can't find a historical reason for it. Basically our current understanding is that on Darwin at least, the most naive implementation (without any __need_FOO macros) should work. Or at least it would be intended to work.

I don't know about Linuxes though. At the very least, as far as libc++ is concerned, I am quite tempted to close this PR and go with #86843 instead.

[ian-twilightcoder]

No one's really sure why it's important for the __need_ macros to be perfectly strict. Trying out #86843 internally. We're still talking about if we want to keep this PR for the tests and copy stddef.h from that one, or dump the tests and just do 86843 by itself.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[ldionne]

We tried out #86843 internally and it did fix the problem we had encountered. Since it's simpler, I'm closing this PR in favor of #86843.