[PATCH] D54479: Mark some library types and templates with visibility+availability attributes.

Louis Dionne via Phabricator reviews at reviews.llvm.org
Tue Nov 13 09:52:35 PST 2018


ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

Answers based on my understanding of how the availability attributes work.



================
Comment at: include/future:531
 
-class _LIBCPP_TYPE_VIS _LIBCPP_AVAILABILITY_FUTURE __assoc_sub_state
+class _LIBCPP_AVAILABILITY_FUTURE __assoc_sub_state
     : public __shared_count
----------------
I don't think it is necessary to mark non API facing types with the availability attribute. So this one does not look right.

Also, we currently export the member functions of `__assoc_sub_state` from the dylib. Removing the `_LIBCPP_TYPE_VIS` means that those would not be exported anymore if we started building with `-fvisibility=hidden`, so removing the visibility attribute does not look right.


================
Comment at: include/future:2301
 template <class _Fp, class... _Args>
-class __async_func
+class _LIBCPP_AVAILABILITY_FUTURE __async_func
 {
----------------
Not an API-facing type.


================
Comment at: include/future:2372
 template <class _Rp>
-class _LIBCPP_TEMPLATE_VIS shared_future
+class _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FUTURE shared_future
 {
----------------
Look OK.


================
Comment at: include/future:2446
 template <class _Rp>
-class _LIBCPP_TEMPLATE_VIS shared_future<_Rp&>
+class _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FUTURE shared_future<_Rp&>
 {
----------------
Looks OK.


================
Comment at: include/shared_mutex:310
 template <class _Mutex>
-class shared_lock
+class _LIBCPP_TEMPLATE_VIS shared_lock
 {
----------------
We don't seem to be exporting anything related to `shared_lock` from the dylib, so I would leave this out.


Repository:
  rCXX libc++

https://reviews.llvm.org/D54479





More information about the libcxx-commits mailing list