This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Add incomplete availability markup for std::pmr
ClosedPublic

Authored by ldionne on Oct 12 2022, 1:50 PM.

Details

Summary

This fixes rdar://110330781, which asked for the feature-test macro
for std::pmr to take into account the deployment target. It doesn't
fix https://llvm.org/PR62212, though, because the availability markup
itself must be disabled until some Clang bugs have been fixed.

This is pretty vexing, however at least everything should work once
those Clang bugs have been fixed. In the meantime, this patch at least
adds the required markup (as disabled) and ensures that the feature-test
macro for std::pmr is aware of the deployment target requirement.

Diff Detail

Event Timeline

philnik created this revision.Oct 12 2022, 1:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 12 2022, 1:50 PM
Herald added a subscriber: krytarowski. · View Herald Transcript
philnik requested review of this revision.Oct 12 2022, 1:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 12 2022, 1:50 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne requested changes to this revision.Oct 13 2022, 9:44 AM
ldionne added inline comments.
libcxx/include/__availability
330

I *think* it does the correct thing, but you should be adding tests to verify that. See for example libcxx/test/libcxx/thread/atomic.availability.verify.cpp.

This revision now requires changes to proceed.Oct 13 2022, 9:44 AM
philnik updated this revision to Diff 468561.Oct 18 2022, 8:28 AM
philnik marked an inline comment as done.
  • Address comments
Mordante added inline comments.
libcxx/include/__memory_resource/unsynchronized_pool_resource.h
74

This function probably needs to be marked too since it calls release.

I usually use the attribute on the entire class/struct.

philnik updated this revision to Diff 468824.Oct 19 2022, 2:28 AM
philnik marked an inline comment as done.
  • Address comments
libcxx/include/__memory_resource/unsynchronized_pool_resource.h
74

Thanks for the info! I didn't know you could mark a class as unavailable.

philnik updated this revision to Diff 468858.Oct 19 2022, 3:53 AM
  • Try to fix CI

I think you need to propagate availability macros to the typedefs in std::pmr. You should also add a simple test for those -- it's probably fine to put everything inside the same file.

libcxx/include/__availability
172

Missing a comment like all the other ones.

philnik updated this revision to Diff 470024.Oct 23 2022, 4:57 PM
  • Address comments
ldionne accepted this revision.Oct 27 2022, 9:25 AM

LGTM once CI is passing. I'll try to get you access to our CI node per our discussion.

This revision is now accepted and ready to land.Oct 27 2022, 9:25 AM
ldionne commandeered this revision.Apr 21 2023, 11:23 AM
ldionne edited reviewers, added: philnik; removed: ldionne.
This revision now requires review to proceed.Apr 21 2023, 11:23 AM
ldionne updated this revision to Diff 515851.Apr 21 2023, 11:24 AM

Rebase onto main. There's a lot of problems that I think are basically Clang bugs with the availability attributes.

ldionne updated this revision to Diff 515862.Apr 21 2023, 11:46 AM
ldionne retitled this revision from [libc++][PMR] Add availability macros to [libc++] Add availability markup for std::pmr.
ldionne edited the summary of this revision. (Show Details)

Update commit message

Mordante accepted this revision.Apr 22 2023, 4:09 AM

LGTM modulo one comment.

libcxx/test/libcxx/utilities/utility/mem.res/pmr.availability.verify.cpp
9

Based on the other test we haven't backported PMR.

This revision is now accepted and ready to land.Apr 22 2023, 4:09 AM
ldionne updated this revision to Diff 524813.May 23 2023, 11:30 AM

Rebase and trigger CI

ldionne marked an inline comment as done.Jun 7 2023, 10:30 AM
ldionne updated this revision to Diff 532155.Jun 16 2023, 8:01 AM
ldionne retitled this revision from [libc++] Add availability markup for std::pmr to [libc++] Add incomplete availability markup for std::pmr.
ldionne edited the summary of this revision. (Show Details)

Disable availability markup and explain why.

ldionne updated this revision to Diff 532237.Jun 16 2023, 10:57 AM

Rebase on top of GCC fix for CI.

ldionne updated this revision to Diff 532678.Jun 19 2023, 8:42 AM

Update failing tests on Apple

This revision was automatically updated to reflect the committed changes.