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

[Louis Dionne via Phabricator via llvm-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

I think the direction works, but I have questions about some thing I don't understand (mostly in the tests).



================
Comment at: libcxx/include/stddef.h:58
     !defined(__DEFINED_max_align_t) && !defined(__NetBSD__)
 typedef long double max_align_t;
 #endif
----------------
Why do we need this at all anymore? In C++11, can't we rely on the C Standard Library providing it `::max_align_t`?


================
Comment at: libcxx/test/libcxx/experimental/memory/memory.resource.adaptor/memory.resource.adaptor.mem/db_deallocate.pass.cpp:40
+#ifdef __STDCPP_DEFAULT_NEW_ALIGNMENT__
+    std::size_t maxSize = std::numeric_limits<std::size_t>::max()
+                            - __STDCPP_DEFAULT_NEW_ALIGNMENT__;
----------------
This test is only enabled in >= C++11, so I think `std::max_align_t` should always be provided. So why do we want that `#if`?


================
Comment at: libcxx/test/std/language.support/support.types/max_align_t.pass.cpp:47
+#ifdef __STDCPP_DEFAULT_NEW_ALIGNMENT__
+    static_assert(std::alignment_of<std::max_align_t>::value <=
+                  __STDCPP_DEFAULT_NEW_ALIGNMENT__,
----------------
Since we now assume that `__STDCPP_DEFAULT_NEW_ALIGNMENT__` is defined, can we remove that `#if` and keep the assertion?


================
Comment at: libcxx/test/std/utilities/meta/meta.trans/meta.trans.other/aligned_storage.pass.cpp:275
+#if TEST_STD_VER >= 11
+    const size_t alignment = TEST_ALIGNOF(std::max_align_t) > 16 ?
+        16 : TEST_ALIGNOF(std::max_align_t);
----------------
Can you walk me through why the check for `> 16` is required?


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

https://reviews.llvm.org/D73245