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

Louis Dionne via Phabricator reviews at reviews.llvm.org
Wed Nov 14 05:02:36 PST 2018


ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


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


Repository:
  rCXX libc++

https://reviews.llvm.org/D54479





More information about the libcxx-commits mailing list