[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in <new>

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added inline comments.


================
Comment at: libcxx/include/new:229-237
+#if !defined(_LIBCPP_CXX03_LANG)
+using __libcpp_max_align_t = max_align_t;
+#else
+union __libcpp_max_align_t {
+  void * __f1;
+  long long int __f2;
+  long double __f3;
----------------
rsmith wrote:
> Is there any ODR risk from this, or similar? Does libc++ support building in mixed C++98 / C++11 mode? If different TUs disagree on this alignment, we can end up allocating with the aligned allocator and deallocating with the unaligned allocator, which is not guaranteed to work.
> 
> We could always use the union approach if we don't know the default new alignment. But from the code below it looks like we might only ever use this if we have aligned allocation support, in which case we can just assume that the default new alignment is defined. So perhaps we can just hide the entire definition of `__is_overaligned_for_new` behind a `#ifdef __STDCPP_DEFAULT_NEW_ALIGNMENT__` and never even consider `max_align_t`?
`max_align_t` should be ABI stable. I would rather we copy/paste the clang definition into the libc++ sources so we can use it when it's not provided by the compiler.

Though this raises another can of worms because GCC and Clang don't agree on a size for `max_align_t`.


================
Comment at: libcxx/include/new:232
+#else
+union __libcpp_max_align_t {
+  void * __f1;
----------------
If we're going to go this route, we should expose the `__libcpp_max_align_t` definition in all dialects, and compare it's size and alignment to the real `max_align_t` when we have it.


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

https://reviews.llvm.org/D73245