This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Add availability markup for aligned new/delete
AbandonedPublic

Authored by ldionne on Aug 2 2018, 3:45 PM.

Details

Reviewers
EricWF
vsapsai
Summary

The aligned allocation and deallocation functions were added in the dylib
in Mac OSX 10.13. This commit does two things: first, it adds availability
markup to those functions such that clients targetting older OSes get a
compiler error instead of a link error if they try to use these functions.

Secondly, it makes sure that the library itself does not use these functions
when targetting older OSes.

rdar://problem/34223934
rdar://problem/42826941

Event Timeline

ldionne created this revision.Aug 2 2018, 3:45 PM

I just wanted to make sure that this doesn't have the same problem as https://reviews.llvm.org/D34556. Is that correct?

The patch was reverted in r306859. https://reviews.llvm.org/D34574#791158 explains why the approach taken in the patch was wrong.

I just wanted to make sure that this doesn't have the same problem as https://reviews.llvm.org/D34556. Is that correct?

The patch was reverted in r306859. https://reviews.llvm.org/D34574#791158 explains why the approach taken in the patch was wrong.

I was neither aware of https://reviews.llvm.org/D34574 nor https://reviews.llvm.org/D34556, but that changes the picture quite a bit. So, IIUC, we want to predicate our use of the aligned allocation functions on whether __cpp_aligned_new is defined, which will be defined correctly because of https://reviews.llvm.org/D45015. Since we're already predicated on that, there's really not much to do in libc++.

I think it's still a good idea to have the availability markup, though.

ldionne updated this revision to Diff 159110.Aug 3 2018, 3:45 PM

Only include availability markup in this patch, do not change whether and when aligned allocation functions are used by the library. This will be handled in a separate patch.

EricWF added a comment.Aug 3 2018, 8:47 PM

How does this work when a user provides their own definitions? Does the attribute from the declaration still produce a warning? If so, then I think an in-compiler approach is better.

How does this work when a user provides their own definitions? Does the attribute from the declaration still produce a warning? If so, then I think an in-compiler approach is better.

Yes. I do agree that not warning when the user provides their own definition is a better user experience, however I think that is already the behavior we have for sized new/delete (with _LIBCPP_AVAILABILITY_SIZED_NEW_DELETE). Is it any different?

If we were to go for an in-compiler approach, what would be the behavior we want? Any TU that uses the operator but defines it would not get a warning, but any TU that uses it without defining it would still get a warning, right? If so, it doesn't seem like such a huge improvement.

ldionne abandoned this revision.Aug 6 2018, 8:14 AM

Nevermind, it looks like this patch is not necessary anymore since https://reviews.llvm.org/D45015 landed.