[libcxx-commits] [PATCH] D114786: [libc++] Remove the ability to use the std::nullptr_t emulation in C++03 mode

[James Nagurne via Phabricator via libcxx-commits]

JamesNagurne added inline comments.


================
Comment at: libcxx/include/stddef.h:48
 #ifdef __cplusplus
-
-extern "C++" {
-#include <__nullptr>
-using std::nullptr_t;
-}
-
+    typedef decltype(nullptr) nullptr_t;
 #endif
----------------
ldionne wrote:
> Quuxplusone wrote:
> > @JamesNagurne @ldionne : Whoa, yeah. Shouldn't this be
> > ```
> > #ifdef __cplusplus
> > namespace std { using nullptr_t = decltype(nullptr); }
> > #endif
> > ```
> > and that's all? Also, what was this ever doing in C's <stddef.h>? It should go in <cstddef> instead.
> > If we want to preserve libc++'s historical behavior, it seems it would be
> > ```
> > // in <cstddef>
> > namespace std { using nullptr_t = decltype(nullptr); }
> > 
> > // in <stddef.h>
> > namespace std { using nullptr_t = decltype(nullptr); }
> > using std::nullptr_t;
> > ```
> > (Yes, including the same `using nullptr_t = decltype(nullptr);` token-sequence twice in a row is actually fine with C++ these days. I checked that at least GCC trunk's C++11 mode and MSVC latest's C++11 mode are happy with it.)
> > But, if we want to do the most sensible thing regardless of backward compatibility, I think it would be
> > ```
> > // in <cstddef>
> > namespace std { using nullptr_t = decltype(nullptr); }
> > 
> > // in <stddef.h>
> > /* nothing at all */
> > ```
> > 
> > I propose to leave this to @ldionne to address, unless he delegates it back to me. :)
> @JamesNagurne @Quuxplusone 
> 
> I agree it's messed up that we are defining `::nullptr_t`, however that was the state of things before this patch too, wasn't it? I don't understand why https://godbolt.org/z/GjbEh1hqT shows different behavior, I must be missing something. Certainly this patch did not intend to change whether we provided `::nullptr_t` or not.
> 
> Edit: OH, now I see it. We've always been defining `::nullptr_t` from `<stddef.h>`, but now we also define it from `<cstddef>`, and that's the behavior change. That was not intended.
> 
> @Quuxplusone are we really allowed to not define `::nullptr_t` at all from `<stddef.h>`?
Not Arthur, but:

C++14 18.2 describes <cstddef>
- 18.2.2: Contents are the same as stddef.h (I suspect this then implies the requirements of C11's stddef.h description), except:
- 18.2.9: nullptr_t is a namespace std typedef to decltype(nullptr)

C11 Annex B.18
- Unsurprisingly, no reference to nullptr_t

C++14 Annex D.5.2, going to quote it to avoid ambiguity:
> Every C header, each of which has a name of the form name.h, behaves as if each name placed in the standard library namespace by the corresponding cname header is placed within the global namespace scope. It is unspecified whether these names are first declared or defined within namespace scope (3.3.6) of the namespace std and are then injected into the global namespace scope by explicit using-declarations (7.3.3).

  - std::nullptr_t defined in cstddef
  - stddef.h places that name into global namespace scope either through injection of std::nullptr_t or through a compatible redefinition

This allows for nullptr_t to be in the global namespace when including stddef.h. However, the standard is pretty clear that <cstddef> shall not define a global nullptr_t name.







Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114786/new/

https://reviews.llvm.org/D114786