[PATCH] D45013: Generate warning when over-aligned new call is required but not supported.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai accepted this revision.
vsapsai added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D45013#1053998, @EricWF wrote:

> In https://reviews.llvm.org/D45013#1052600, @vsapsai wrote:
>
> > Does it make sense to have a warning for `__libcpp_deallocate` and `get_temporary_buffer` too? Not sure about deallocate as you have allocated memory already and allocation has a warning. So for deallocation a warning can be too noisy. But for `get_temporary_buffer` the warning seems reasonable, unless I'm missing something.
>
>
> `get_temporary_buffer` shouldn't return a temporary buffer when the request is over-aligned and aligned allocation isn't available; so I don't think we should warn on it.


OK. Makes sense. We don't have data indicating it is a widespread issue that developers forget to address, so no need for the warning. Also `get_temporary_buffer` is deprecated in C++17, so there is not much value in investing into it.

Overall the change looks good to me, just a few minor NFC tweaks.



================
Comment at: include/memory:2030
 #else
-    if (__is_overaligned_for_new(__alignof(_Tp)))
+    if (_LIBCPP_IS_OVERALIGNED_FOR_NEW(__alignof(_Tp)))
         {
----------------
Is the indentation for these 2 ifs correct? They seem to be aligned with `#if` and not with `while`. Don't know if that is intentional.


================
Comment at: include/new:235-239
 #ifdef __STDCPP_DEFAULT_NEW_ALIGNMENT__
-  return __align > __STDCPP_DEFAULT_NEW_ALIGNMENT__;
+#define _LIBCPP_IS_OVERALIGNED_FOR_NEW(__align) __align > __STDCPP_DEFAULT_NEW_ALIGNMENT__
 #else
-  return __align > alignment_of<max_align_t>::value;
+#define _LIBCPP_ISOVERALIGNED_FOR_NEW(__align) __align > _VSTD::alignment_of<_VSTD::max_align_t>::value
 #endif
----------------
Seems non-critical, maybe put `__align` in parentheses like `(__align) > ...`? But I don't know what is libc++ style regarding macro arguments.


Repository:
  rCXX libc++

https://reviews.llvm.org/D45013