[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In https://reviews.llvm.org/D45015#1064930, @EricWF wrote:

> In https://reviews.llvm.org/D45015#1064922, @vsapsai wrote:
>
> > Another approach is `__has_feature` but I don't think it is applicable in this case.
> >
> > Is there a way right now to detect that aligned allocation is supported by clang, regardless of link time? Asking to make sure we are consistent.
>
>
> There is a C++ feature test macro, as specified by the standard, `__cpp_aligned_new`.  But I'm not sure how to be consistent with that.


After more thinking I believe a predefined macro is the best option. And in spirit it is consistent with SD-6: SG10 Feature Test Recommendations <https://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations>.

As for the naming, I see it is consistent with `-faligned-alloc-unavailable` but for me it is hard to tell immediately if "unavailable" means "unsupported at all" or "supported but unavailable on the target". Maybe it is just me and others don't have such problem but probably including "runtime" in the macro name would help. Something like `__ALIGNED_ALLOCATION_UNAVAILABLE_RUNTIME__`.

For the overall aligned allocation plan overall. We also need to keep in mind users who provide their own aligned allocation / deallocation functions, so they can still do that. So far everything seems to be OK but we need to be careful.



================
Comment at: lib/Frontend/InitPreprocessor.cpp:1059-1060
 
+  if (!LangOpts.AlignedAllocation || LangOpts.AlignedAllocationUnavailable)
+    Builder.defineMacro("__ALIGNED_ALLOCATION_UNAVAILABLE__");
+
----------------
Don't know what the macro will be in the end, please consider adding a comment clarifying runtime availability.


================
Comment at: test/Preprocessor/predefined-macros.c:288
+// RUN: %clang_cc1 %s -x c++ -E -dM -faligned-allocation -faligned-alloc-unavailable  \
+// RUN: | FileCheck %s -match-full-lines -check-prefix=CHECK-ALIGNED-ALLOC-UNAVAILABLE
+
----------------
Would it be useful to test `-fno-aligned-allocation` too? Extensive testing in different combinations doesn't add much value but `-std=c++17 -fno-aligned-allocation` can be useful.


Repository:
  rC Clang

https://reviews.llvm.org/D45015