Mark some library types and templates with visibility+availability attributes.

[• Quuxplusone]

Summary

I asked: "Why aren't these class types and templates marked with the seemingly appropriate attributes? Should they be?"
@EricWF said: "Add @ldionne to the review. Apple is the one who cares about availability."

[ldionne]

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

include/future

531

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.

2301

Not an API-facing type.

2372

Look OK.

2446

Looks OK.

include/shared_mutex

310

We don't seem to be exporting anything related to shared_lock from the dylib, so I would leave this out.

[• Quuxplusone]

include/shared_mutex

310

The dylib doesn't have any symbols related to deque, either, right? Yet libc++ does

class _LIBCPP_TEMPLATE_VIS deque

I guess I still want the simple rule to be "if it's part of the library's public API _or_ mangled into the dylib, it gets a visibility attribute."
If the visibility attributes are really only for things mangled into the dylib, then a whole lot of things could safely lose their visibility attributes — basically most of the STL. And then there wouldn't really be any consistent rule; it'd just be "don't write a visibility attribute on anything until someone reports that it's broken, and then add one exactly where it's needed." But that sounds impossibly hard to get right. I'd still rather make a rule, even if it results in some "extra" annotation.

[• Quuxplusone]

Re-add _LIBCPP_TYPE_VIS to __assoc_sub_state because it is mentioned in the mangled names of functions exported from src/future.cpp.
Remove _LIBCPP_AVAILABILITY_FUTURE from types that aren't supposed to be visible to user-programmers anyway (IIUC).

[ldionne]

include/shared_mutex

310

For non-template types, no visibility attribute should mean it is hidden. For templates, default visibility could be used to allow users to explicitly instantiate templates in their dylibs and have those be visible. However, we should not support that as they could also apply an attribute to the explicit instantiation, which is cleaner.

In the future, we want no visibility annotations at all except on the few things we export from the dylib. And hence this one would not have an attribute.

[• Quuxplusone]

Discussed offline with @ldionne and my verdict was "If there is any rule for adding these attributes, then the current codebase doesn't follow that rule very well." So, abandoning; and if there's any ad-hoc fix to be made in the areas touched by this patch, then Louis can make it.

include/future

 #ifndef _LIBCPP_NO_EXCEPTIONS
     throw future_error(make_error_code(_Ev));
 #else
     ((void)_Ev);
     _VSTD::abort();
 #endif
 }
 
-class _LIBCPP_TYPE_VIS _LIBCPP_AVAILABILITY_FUTURE __assoc_sub_state
+class _LIBCPP_TYPE_VIS __assoc_sub_state
     : public __shared_count
 {
 protected:
     exception_ptr __exception_;
     mutable mutex __mut_;
     mutable condition_variable __cv_;
     unsigned __state_;
 

 {
     unique_ptr<__async_assoc_state<_Rp, _Fp>, __release_shared_count>
         __h(new __async_assoc_state<_Rp, _Fp>(_VSTD::forward<_Fp>(__f)));
     _VSTD::thread(&__async_assoc_state<_Rp, _Fp>::__execute, __h.get()).detach();
     return future<_Rp>(__h.get());
 }
 
 template <class _Fp, class... _Args>
 class __async_func
 {
     tuple<_Fp, _Args...> __f_;
 
 public:
     typedef typename __invoke_of<_Fp, _Args...>::type _Rp;
 
     _LIBCPP_INLINE_VISIBILITY
     explicit __async_func(_Fp&& __f, _Args&&... __args)
return _VSTD::async(launch::any, _VSTD::forward<_Fp>(__f),
                                     _VSTD::forward<_Args>(__args)...);
 }
 
 #endif  // _LIBCPP_HAS_NO_VARIADICS
 
 // shared_future
 
 template <class _Rp>
-class _LIBCPP_TEMPLATE_VIS shared_future
+class _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FUTURE shared_future
 {
     __assoc_state<_Rp>* __state_;
 
 public:
     _LIBCPP_INLINE_VISIBILITY
     shared_future() _NOEXCEPT : __state_(nullptr) {}
     _LIBCPP_INLINE_VISIBILITY
     shared_future(const shared_future& __rhs)  _NOEXCEPT : __state_(__rhs.__state_)
if (__rhs.__state_)
         __rhs.__state_->__add_shared();
     if (__state_)
         __state_->__release_shared();
     __state_ = __rhs.__state_;
     return *this;
 }
 
 template <class _Rp>
-class _LIBCPP_TEMPLATE_VIS shared_future<_Rp&>
+class _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FUTURE shared_future<_Rp&>
 {
     __assoc_state<_Rp&>* __state_;
 
 public:
     _LIBCPP_INLINE_VISIBILITY
     shared_future() _NOEXCEPT : __state_(nullptr) {}
     _LIBCPP_INLINE_VISIBILITY
     shared_future(const shared_future& __rhs) : __state_(__rhs.__state_)

include/shared_mutex

shared_timed_mutex::try_lock_shared_until(
     }
     unsigned __num_readers = (__base.__state_ & __base.__n_readers_) + 1;
     __base.__state_ &= ~__base.__n_readers_;
     __base.__state_ |= __num_readers;
     return true;
 }
 
 template <class _Mutex>
-class shared_lock
+class _LIBCPP_TEMPLATE_VIS shared_lock
 {
 public:
     typedef _Mutex mutex_type;
 
 private:
     mutex_type* __m_;
     bool __owns_;