[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.
Akira Hatanaka via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue May 22 22:29:00 PDT 2018
ahatanak added a comment.
In https://reviews.llvm.org/D45015#1105314, @rsmith wrote:
> Hmm, perhaps our strategy for handling aligned allocation on Darwin should be revisited. We shouldn't be defining `__cpp_aligned_allocation` if we believe it doesn't work -- that will break code that uses aligned allocation where available and falls back to something else where it's unavailable.
> @ahatanak: how about this:
> - Change the driver to not pass `-faligned-alloc-unavailable` if an explicit `-faligned-allocation` or `-fno-aligned-allocation` flag is given; update Clang's note to suggest explicitly passing `-faligned-allocation` rather than `-Wno-aligned-alloc-unavailable` if the user provides their own aligned allocation function.
> - Change `-faligned-alloc-unavailable` so that it does not define `__cpp_aligned_allocation`.
> - Change Sema's handling of the "aligned allocation unavailable" case so that, after warning on selecting an aligned allocation function, it then removes those functions from the candidate set and tries again.
> That is: on old Darwin, we should not define `__cpp_aligned_allocation` (even in C++17), produce the "no aligned allocation support" warning in C++17 mode, and then not try to call the aligned allocation function. But if `-faligned-allocation` or `-fno-aligned-allocation` is specified explicitly, then the user knows what they're doing and they get no warning.
I talked with @vsapsai today and we think this looks like a better idea.
A couple of questions:
- Currently clang errors out when aligned operator new is selected but the OS's version is too old to support it. What's the reason we want to change this now to be a warning rather than an error?
- So clang no longer needs to define macro `__ALIGNED_ALLOCATION_UNAVAILABLE__` and libc++ will use `__cpp_aligned_new` (I think you meant `__cpp_aligned_new`, not `__cpp_aligned_allocation`?) to determine whether aligned allocation functions should be defined or made available in the header?
More information about the cfe-commits