This is an archive of the discontinued LLVM Phabricator instance.

Add markup for libc++ dylib availability
ClosedPublic

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

Details

Summary

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 <https://clang.llvm.org/docs/AttributeReference.html#availability>_
that can be placed on declarations to describe the lifecycle of a symbol in the
library.

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

Event Timeline

mehdi_amini created this revision.Apr 5 2017, 8:04 PM
jroelofs added inline comments.
utils/libcxx/test/config.py
289

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

316

Why does the availability stuff clash with -verify?

363

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

387

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.
include/__config
1160

Redundant whitespace added?

utils/libcxx/test/config.py
358

Typo: add_deployment_feature

mehdi_amini marked 5 inline comments as done.Apr 20 2017, 10:48 PM
mehdi_amini added inline comments.
utils/libcxx/test/config.py
289

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

316

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

358

French style :)

363

No, just bad copy/paste :)

387

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
utils/libcxx/test/config.py
387

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.

libcxx/trunk/utils/libcxx/test/config.py
400 ↗(On Diff #97849)

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 a subscriber: thakis.Mar 11 2019, 8:13 AM
thakis added inline comments.
libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/lit.local.cfg
1 ↗(On Diff #97849)

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.
libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/lit.local.cfg
1 ↗(On Diff #97849)

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
libcxx/trunk/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/lit.local.cfg
1 ↗(On Diff #97849)

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.