This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Fix and document visibility attributes for Clang, GCC and Windows.
ClosedPublic

Authored by EricWF on Sep 15 2016, 2:10 AM.

Details

Summary

This patch fixes a number of problems with the visibility macros across GCC (on Unix) and Windows (DLL import/export semantics). All of the visibility macros are now documented under DesignDocs/VisibilityMacros.rst. Now I'll no longer forget the subtleties of each!

This patch adds two new visibility macros:

  • _LIBCPP_ENUM_VIS for controlling the typeinfo of enum types. Only Clang supports this.
  • _LIBCPP_EXTERN_TEMPLATE_TYPE_VIS for redefining visibility on explicit instantiation declarations. Clang and Windows require this.

After applying this patch GCC only emits one -Wattribute warning opposed to 30+.

Diff Detail

Event Timeline

EricWF updated this revision to Diff 71483.Sep 15 2016, 2:10 AM
EricWF retitled this revision from to [libc++] Fix and document visibility attributes for Clang, GCC and Windows..
EricWF updated this object.
EricWF added a reviewer: mclow.lists.
EricWF added a subscriber: cfe-commits.
EricWF updated this revision to Diff 71486.Sep 15 2016, 2:26 AM

Fix obvious errors in docs.

include/__config
605

Checking __type_visibility__ and using __visibility__ looks weird but is intended as it's meant to override __type_visibility__.

include/__string
704–705

This suppresses the GCC diagnostic warning: always_inline function might not be inlinable [-Wattributes] .

EricWF updated this revision to Diff 71570.Sep 15 2016, 3:32 PM

I'm going to go ahead and commit this w/o review because it's blocking more important changes. Post-commit comments on the documentation are appreciated.

EricWF accepted this revision.Sep 15 2016, 3:32 PM
EricWF added a reviewer: EricWF.
This revision is now accepted and ready to land.Sep 15 2016, 3:32 PM
EricWF closed this revision.Sep 15 2016, 3:35 PM
smeenai added inline comments.
docs/DesignDocs/VisibilityMacros.rst
33

The _LIBCPP_TYPE_VIS at the start of this line seems out of place?

include/__config
605

Do you think it's worth adding a comment in the code to this effect, or even just a comment pointing to the visibility macros design doc? It's a bit confusing. Alternatively, maybe define a _LIBCPP_OVERRIDE_TYPE_VIS inside the __has_attribute(__type_visibility__) case of _LIBCPP_TYPE_VIS and use that macro here instead of repeating the type visibility attribute check, to make this self documenting.