[PATCH] D54814: Move internal usages of `alignof`/`__alignof` to use `_LIBCPP_ALIGNOF`.

[Eric Fiselier via Phabricator]

EricWF accepted this revision.
EricWF marked 6 inline comments as done.
EricWF added a comment.
This revision is now accepted and ready to land.

> Please hold off with committing this until we've agreed on the ABI part. That'll take at least until next week when people are back from Thanksgiving.

I don't know if there is an ABI part to agree on. Libc++ should have always been using `alignof`. The fact we were using `__alignof` was unobservable until Clang 8. If we release this in libc++ 8, it will continue to appear as if we have always been using `alignof`. 
If users want the old behavior, the use the Clang flag.

Accepting. Further review can be post-commit review.



================
Comment at: include/__config:1285
+#ifndef _LIBCPP_CXX03_LANG
+#define _LIBCPP_ALIGNOF(_Tp) alignof(_Tp)
+#elif defined(_LIBCPP_COMPILER_CLANG)
----------------
ldionne wrote:
> Can you indent nested `#define`s? I find it very difficult to read our PP code because it often lacks indentation.
OK, but generally I'm pretty hostile to manually formatting anything. If `clang-format` doesn't do it, then I don't normally care.


================
Comment at: test/libcxx/libcpp_alignof.pass.cpp:29
+#ifdef TEST_COMPILER_CLANG
+  static_assert(_LIBCPP_ALIGNOF(T) == _Alignof(T), "");
+#endif
----------------
ldionne wrote:
> Why is this test useful? Isn't the previous one (for C++11) sufficient?
It checks that `_Alignof` acts like `alignof` which acts like `_LIBCPP_ALIGNOF`. The test is useful in that it would have caught the Clang bug related to this change.

And additional tests never hurt. This was certainly doesn't. 


================
Comment at: test/support/test_macros.h:118
 #else
-#define TEST_ALIGNOF(...) __alignof(__VA_ARGS__)
+#if defined(TEST_COMPILER_CLANG)
+# define TEST_ALIGNOF(...) _Alignof(__VA_ARGS__)
----------------
ldionne wrote:
> Why not always just `alignof`? Shouldn't we only use that macro in C++11 and above anyway?
We provide almost all of the C++11 library in C++03, C++03 is really just a thing we tolerate. For example, we provide `alignment_of` in C++03. Our allocators pass around the alignment in case we can use aligned new, etc. 


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D54814