Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ Bug Fixes in This Version
cast chain. (#GH149967).
- Fixed a crash with incompatible pointer to integer conversions in designated
initializers involving string literals. (#GH154046)
- Clang's ``<stddef.h>`` now properly declares ``nullptr_t`` in C++ mode. (#GH154577).

Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
3 changes: 2 additions & 1 deletion clang/lib/Headers/__stddef_nullptr_t.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
#define _NULLPTR_T

#ifdef __cplusplus
#if defined(_MSC_EXTENSIONS) && defined(_NATIVE_NULLPTR_SUPPORTED)
#if __cplusplus >= 201103L || \
(defined(_MSC_EXTENSIONS) && defined(_NATIVE_NULLPTR_SUPPORTED))
Comment on lines +19 to +20
Copy link
Contributor

Choose a reason for hiding this comment

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

libc++ provides nullptr_t in all language modes. It would be really nice if we could remove our stddef.h header, but I'm not sure how the Clang headers would detect whether to provide it or not in C++03.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know that it's possible to not vend stddef.h; do we know that every libc actually provides a complete header that works with Clang?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or is your suggestion that we move stddef.h to something more like:

#if defined(__cplusplus) && __cplusplus >= 201103L /* c++11 */
#include_next <stddef.h>
#else
/* The usual stddef.h header */
#endif

Either way, I think it's orthogonal to this fix because that's a pretty substantial change that requires a fair amount of investigation. Are you okay with landing the changes as-is and doing that exploration in a follow-up if someone gets the chance? (I'm going to be out of office for a while, so I'm not able to commit to that work myself.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I meant libc++'s stddef.h with "our stddef.h". We currently have a wrapping header to provide nullptr_t correctly: https://godbolt.org/z/W41dKKbd4.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah! I see what you mean, thanks!

So you're okay with the current changes as-is because it heads in the right direction for libc++?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, I'm OK with it. I just wish we could go all the way.

+1, but it might be a challenge too. I'm not certain libc++ should provide nullptr_t in C++03 mode; that's taking an identifier from the user in that language mode and nullptr doesn't work in C++03 anyway: https://godbolt.org/z/96vMdbobe

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, I'm OK with it. I just wish we could go all the way.

+1, but it might be a challenge too. I'm not certain libc++ should provide nullptr_t in C++03 mode;

We claim to be a C++11 implementation, not a C++03 implementation. Just one that mostly works with -std=c++03. Whether that was the right thing to do is debatable, but it's where we are today.

that's taking an identifier from the user in that language mode and nullptr doesn't work in C++03 anyway: https://godbolt.org/z/96vMdbobe

Well, it does with libc++.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We claim to be a C++11 implementation, not a C++03 implementation.

That's not how I understood the RFC, unless I missed something. I understood it as libc++ is a C++03 and later implementation but that we're not making fixes to C++03 unless there's some major security vulnerability or other extenuating circumstance.

Well, it does with libc++.

It's a non-conforming extension.

Copy link
Contributor

Choose a reason for hiding this comment

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

We claim to be a C++11 implementation, not a C++03 implementation.

That's not how I understood the RFC, unless I missed something. I understood it as libc++ is a C++03 and later implementation but that we're not making fixes to C++03 unless there's some major security vulnerability or other extenuating circumstance.

This has nothing to do with the RFC. libc++ has always provided a lot of "extensions" in C++03. From the very beginning we've had C++11 features implemented in C++03. I actually think it probably was a huge mistake, but we can't change it 15 years later. This is one of the main reasons we want to split the headers, so we can leave the cruft behind and not have massive workarounds for all the stuff we provide in 03.

Well, it does with libc++.

It's a non-conforming extension.

If you look at it as a C++03 implementation. If you look at it as a C++11 implementation that works with C++03 compiler it's not.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you look at it as a C++03 implementation. If you look at it as a C++11 implementation that works with C++03 compiler it's not.

Yeah, I'm viewing it from the perspective of a user of Clang. This is non-conforming and has surprising behavior as a result: https://godbolt.org/z/9qPYPndKb

But, it sounds like this is moot because:

but we can't change it 15 years later.

is definitely true. :-)

namespace std {
typedef decltype(nullptr) nullptr_t;
}
Expand Down
26 changes: 25 additions & 1 deletion clang/test/Headers/stddefneeds.cpp
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify -Wsentinel -std=c++11 %s
// RUN: %clang_cc1 -fsyntax-only -triple x86_64-apple-macosx10.9.0 -verify=old,expected -Wsentinel -std=c++98 %s

ptrdiff_t p0; // expected-error{{unknown}}
size_t s0; // expected-error{{unknown}}
void* v0 = NULL; // expected-error{{undeclared}}
wint_t w0; // expected-error{{unknown}}
max_align_t m0; // expected-error{{unknown}}
nullptr_t n0; // expected-error {{unknown}}

#define __need_ptrdiff_t
#include <stddef.h>
Expand All @@ -14,6 +16,7 @@ size_t s1; // expected-error{{unknown}}
void* v1 = NULL; // expected-error{{undeclared}}
wint_t w1; // expected-error{{unknown}}
max_align_t m1; // expected-error{{unknown}}
nullptr_t n1; // expected-error{{unknown}}

#define __need_size_t
#include <stddef.h>
Expand All @@ -23,6 +26,16 @@ size_t s2;
void* v2 = NULL; // expected-error{{undeclared}}
wint_t w2; // expected-error{{unknown}}
max_align_t m2; // expected-error{{unknown}}
nullptr_t n2; // expected-error{{unknown}}

#define __need_nullptr_t
#include <stddef.h>
ptrdiff_t p6;
size_t s6;
void* v6 = NULL; // expected-error{{undeclared}}
wint_t w6; // expected-error{{unknown}}
max_align_t m6; // expected-error{{unknown}}
nullptr_t n6; // old-error{{unknown}}

#define __need_NULL
#include <stddef.h>
Expand All @@ -32,6 +45,16 @@ size_t s3;
void* v3 = NULL;
wint_t w3; // expected-error{{unknown}}
max_align_t m3; // expected-error{{unknown}}
nullptr_t n3; // old-error{{unknown}}

#define __need_max_align_t
#include <stddef.h>
ptrdiff_t p7;
size_t s7;
void* v7 = NULL;
wint_t w7; // expected-error{{unknown}}
max_align_t m7;
nullptr_t n7; // old-error{{unknown}}

// Shouldn't bring in wint_t by default:
#include <stddef.h>
Expand All @@ -41,6 +64,7 @@ size_t s4;
void* v4 = NULL;
wint_t w4; // expected-error{{unknown}}
max_align_t m4;
nullptr_t n4; // old-error{{unknown}}

#define __need_wint_t
#include <stddef.h>
Expand All @@ -50,7 +74,7 @@ size_t s5;
void* v5 = NULL;
wint_t w5;
max_align_t m5;

nullptr_t n5; // old-error{{unknown}}

// linux/stddef.h does something like this for cpp files:
#undef NULL
Expand Down
Loading