[PATCH] D46443: [libc++] Add missing cstdalign header

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: libcxx/include/cstdalign:24
+#include <__config>
+#include <stdalign.h>
+
----------------
sbc100 wrote:
> hubert.reinterpretcast wrote:
> > This seems to be assuming that the underlying C library's `stdalign.h` is C++ friendly. A C11 `stdalign.h` //does// define `alignof` and `alignas` as macros.
> Should I just remove this `#include` then?
The idea would be to //add// a `stdalign.h` alongside this header that doesn't `#include_next` the underlying C library's `stdalign.h`.


================
Comment at: libcxx/test/std/language.support/cstdalign/cstdalign.pass.cpp:27
+#ifndef __alignof_is_defined
+#error __alignof_is_defined not defined
+#endif
----------------
sbc100 wrote:
> hubert.reinterpretcast wrote:
> > sbc100 wrote:
> > > ldionne wrote:
> > > > I'm not seeing `__alignof_is_defined` anywhere in the spec?
> > > Removed
> > Seems like a defect in the old standard. The prose doesn't match the synopsis. `__alignof_is_defined` is a macro in C11's `stdalign.h` (and so is `alignof`). That the C++ committee did not intend for an `alignof` macro can probably be assumed. I suspect the lack of an `__alignof_is_defined` macro was also unintended.
> So should I add back the check for `__alignof_is_defined`?
I think so (in addition to also checking that `alignof` is not defined as a macro). I think the patch needs to be confirmed again either way by the libc++ approvers though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D46443