This is an archive of the discontinued LLVM Phabricator instance.

[libcxx] Annotate c++17 aligned new/delete operators with availability attribute
ClosedPublic

Authored by ahatanak on Jun 23 2017, 7:59 AM.

Details

Summary

This is needed because older versions of libc++ do not have these operators. If users target an older deployment target and try to compile programs in which these operators are explicitly called, the compiler will complain.

The following is the list of minimum deployment targets for the four OSes:

macosx: 10.13
ios: 11.0
tvos: 11.0
watchos: 4.0

I followed the other availability macros in using "strict", but I'm not sure this was the right decision. The documentation seems to recommend not using "strict" (it says that weakly-linking is almost always a better API choice).

rdar://problem/32664169

Diff Detail

Repository
rL LLVM

Event Timeline

ahatanak created this revision.Jun 23 2017, 7:59 AM
erik.pilkington added inline comments.
test/std/language.support/support.dynamic/new.delete/new.delete.placement/new_deployment.fail.cpp
35 ↗(On Diff #103693)

You probably shouldn't include the line numbers here, it makes the test fragile. Ie, do expected-note@new:*

dexonsmith edited edge metadata.Jun 23 2017, 8:35 AM

I followed the other availability macros in using "strict", but I'm not sure this was the right decision. The documentation seems to recommend not using "strict" (it says that weakly-linking is almost always a better API choice).

libc++ is one of the exceptions. Use strict.

dexonsmith added inline comments.Jun 23 2017, 9:03 AM
test/std/language.support/support.dynamic/new.delete/new.delete.placement/new_deployment.fail.cpp
12–16 ↗(On Diff #103693)

There is a lit configuration to choose whether to run with availability or not. We should just mark the test unsupported when we don't want it to run.

It's also important to run the rest of the suite with availability turned on, to see which other tests start to fail.

21–26 ↗(On Diff #103693)

Similarly here, we can just mark the test unsupported when we don't have availability turned on or we're targeting 10.13 or later.

ahatanak updated this revision to Diff 103880.Jun 25 2017, 5:01 PM
ahatanak marked an inline comment as done.

Address review comments.

test/std/language.support/support.dynamic/new.delete/new.delete.placement/new_deployment.fail.cpp
12–16 ↗(On Diff #103693)

I added "REQUIRES: availability=macosx10.12" to avoid running this test when availability is turned off or we are targeting 10.13 or later.

I ran the tests with availability turned on and they all passed.

ahatanak updated this revision to Diff 103883.Jun 25 2017, 6:26 PM

Fix line number and remove #else.

This revision is now accepted and ready to land.Jun 26 2017, 9:57 AM
This revision was automatically updated to reflect the committed changes.

This doesn't work when running the libc++ tests against a non-system libc++. (which is what all the libc++ developers and AND all the test bots do)

I'm not exactly sure why the test failed, but the patch was reverted in r306859.

I'm not exactly sure why the test failed, but the patch was reverted in r306859.

My bad, since the tests are failing due to clang whining about the system dylib not supporting aligned new/delete on 10.12, I meant to post this on D34574