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

[Joerg Sonnenberger via Phabricator via llvm-commits]

joerg marked 2 inline comments as done.
joerg added inline comments.


================
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);
----------------
ldionne wrote:
> joerg wrote:
> > ldionne wrote:
> > > Can you walk me through why the check for `> 16` is required?
> > If max_align_t is 256bit, we still only expect 128bit alignment (long double). This test is still checking more than it should, e.g. in principle a target could legitimately have no natural type larger than 64bit, but support 256bit vector types and set max_align_t accordingly. The condition is here because std::min in C++11 is not constexpr.
> Naively, I would have expected the test to be:
> 
> ```
> #if TEST_STD_VER >= 11 // we know max_align_t is provided
>   static_assert(std::alignment_of<T1>::value == TEST_ALIGNOF(std::max_align_t), "");
> #else
>   static_assert(std::alignment_of<T1>::value >= TEST_ALIGNOF(natural_alignment), "");
> #endif
> ```
> 
> To make sure I understand, you're saying we can't do that because the `std::alignment_of<T1>::value == TEST_ALIGNOF(std::max_align_t)` test would sometimes fail if `alignof(std::max_align_t)` is larger than the natural alignment used by `std::aligned_storage`. This also highlights that there's a change in the value of `alignof(std::max_align_t)` with this change. Is that correct?
Yes, the test would have failed before when `max_align_t > 16`. There is no change of value in `alignof(max_align_t)`, just seen by review.


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

https://reviews.llvm.org/D73245