Page MenuHomePhabricator

Add markup for libc++ dylib availability

Authored by mehdi_amini on Apr 5 2017, 8:04 PM.



Libc++ is used as a system library on macOS and iOS (amongst others). In order
for users to be able to compile a binary that is intended to be deployed to an
older version of the platform, clang provides the
availability attribute <>_
that can be placed on declarations to describe the lifecycle of a symbol in the

See docs/DesignDocs/AvailabilityMarkup.rst for more information.

Diff Detail


Event Timeline

mehdi_amini created this revision.Apr 5 2017, 8:04 PM
jroelofs added inline comments.
289 ↗(On Diff #94316)

Can you expand on what the FIXME here wants? Is there something that AvailabilityMarkup.rst doesn't cover?

316 ↗(On Diff #94316)

Why does the availability stuff clash with -verify?

363 ↗(On Diff #94316)

This line, and the one above it are the same. Is that intentional?

387 ↗(On Diff #94316)

Is it worth filtering out none and unknown, as they're often repeated, and you can't tell which part of the triple they came from?

Consider: arm-none-linux-gnueabi vs arm-none-none-eabi.

Or would it be better to include some marker in the features to say which part of the triple it came from, eg:

- with_system_cxx_lib=arch:arm
- with_system_cxx_lib=vendor:none
- with_system_cxx_lib=os:linux
- with_system_cxx_lib=sys:gnueabi
arphaman added inline comments.
1160 ↗(On Diff #94316)

Redundant whitespace added?

358 ↗(On Diff #94316)

Typo: add_deployment_feature

mehdi_amini marked 5 inline comments as done.Apr 20 2017, 10:48 PM
mehdi_amini added inline comments.
289 ↗(On Diff #94316)

I wrote the FIXME before writing the doc. I added a link to the doc instead.

316 ↗(On Diff #94316)

The syntax verification phase isn't expecting too see compile errors from the availability.

358 ↗(On Diff #94316)

French style :)

363 ↗(On Diff #94316)

No, just bad copy/paste :)

387 ↗(On Diff #94316)

I like this idea!
It seems like orthogonal to the availability introduced here, so it'd belong to a separate patch I think.

mehdi_amini marked 4 inline comments as done.

Address comments from @jroelofs and @arphaman

jroelofs added inline comments.Apr 21 2017, 8:15 AM
387 ↗(On Diff #94316)

sounds fine to me!

@mclow.lists you mentioned on IRC you may have some suggestions to make this less noisy? Do you still plan to review?

@mclow.lists you mentioned on IRC you may have some suggestions to make this less noisy? Do you still plan to review?

dexonsmith accepted this revision.Apr 29 2017, 9:48 PM

This LGTM, and it's liable to bitrot if it hangs out here any longer. We can always iterate in tree if we find a better way to organize the markup and/or tests.

Eric and Marshall: do you have any objection to this being committed now, so we can get some bots up?

This revision is now accepted and ready to land.Apr 29 2017, 9:48 PM
This revision was automatically updated to reflect the committed changes.
bcain added a subscriber: bcain.Nov 27 2017, 3:06 PM

I think some of the XFAIL: availability may be wrong here. I'd submit a patch, but it's not clear to me what the appropriate fix is.


This means that 'availability' is present if use_system_cxx_lib even on platforms other than OSX. It's not clear to me whether or not this was intended.

If this was intended then it's probably wrong to "XFAIL: availability" in some of these tests. This means that these tests went from PASS to XFAIL when "use_system_cxx_lib". If perhaps these tests cannot be used without just-built libcxx, then maybe it makes more sense to mark them "UNSUPPORTED: availability"?

thakis added inline comments.

Did you mean not in here? I don't understand why we want to skip this tests is availability annotations are present.

mehdi_amini marked an inline comment as done.Mar 11 2019, 9:13 AM
mehdi_amini added subscribers: ldionne, Bigcheese.
mehdi_amini added inline comments.

Thanks for looking into this, unfortunately I don't remember the context from 2 years ago. It may have been a typo indeed.

@ldionne and @Bigcheese may be able to help here!

ldionne added inline comments.Mar 11 2019, 9:39 AM

Hmm, I wasn't aware of this. I guess this is because shared_*** used to be marked as unconditionally unavailable, but that changed. I'll figure out when we can enable these tests.