Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc++] Simplify the implementation of <stddef.h> #86843

Merged
merged 1 commit into from
Apr 2, 2024
Merged

[libc++] Simplify the implementation of <stddef.h> #86843

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 8 additions & 17 deletions libcxx/include/stddef.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,6 @@
//
//===----------------------------------------------------------------------===//

#if defined(__need_ptrdiff_t) || defined(__need_size_t) || defined(__need_wchar_t) || defined(__need_NULL) || \
defined(__need_wint_t)

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

# include_next <stddef.h>

#elif !defined(_LIBCPP_STDDEF_H)
# define _LIBCPP_STDDEF_H

/*
stddef.h synopsis

Expand All @@ -36,16 +24,19 @@

*/

# include <__config>
#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>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our builds now fail with

In file included from ../../base/allocator/partition_allocator/src\partition_alloc/partition_alloc_base/debug/alias.h:8:
In file included from ../../third_party/libc++/src/include\cstddef:42:
../../third_party/libc++/src/include\stddef.h(31,2): error: #include_next is a language extension [-Werror,-Wgnu-include-next]
   31 | #include_next <stddef.h>
      |  ^
1 error generated.

…presumably because this is now before the #pragma GCC system_header below. Was that an intentional change?

(https://ci.chromium.org/ui/p/chromium/builders/try/win-rel/560031/overview)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, no that's not intentional. Is -Wgnu-include-next enabled by default? Why did we not see this anywhere else?

I can fix that but it would be nice to have a test that fails if we break this again in the future.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#88214

Copy link
Contributor

@nico nico Apr 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

Is -Wgnu-include-next enabled by default?

No. Apparently someone over here thought it'd be good to compile some of our files with -pedantic, which enables -Wgnu. (Other users of libc++ probably do this too.)

Why did we not see this anywhere else?

I don't know how libc++ tests run. Maybe there isn't a test that includes all public headers and builds with -Wall -Wconversion -Wextra -pedantic -Werror? That would be a good test to have.

Does libc++ have any policy which warning levels it's clean at? Probably not really needed since all its headers are supposed to be system headers which disable warnings, so that test could (and probably should?) even use -Weverything with clang.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#88441


#ifndef _LIBCPP_STDDEF_H
# define _LIBCPP_STDDEF_H

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

# if __has_include_next(<stddef.h>)
# include_next <stddef.h>
# endif

# ifdef __cplusplus
typedef decltype(nullptr) nullptr_t;
# endif
Expand Down
Loading