[PATCH] D35326: [libc++] Add _LIBCPP_TEMPLATE_VIS to string operator+ and __vector_base_common

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added a comment.

In https://reviews.llvm.org/D35326#808855, @thomasanderson wrote:

> Also, string has a class almost identical to __vector_base_common:
>  https://reviews.llvm.org/diffusion/L/browse/libcxx/trunk/include/string;307970$560
>
> But currently string has _LIBCPP_TEMPLATE_VIS and vector does not.  Maybe at least one of them is wrong?


Hmm.

I feel that `__string_base_common` shouldn't be marked `_LIBCPP_TEMPLATE_VIS`. In the clang world, `_LIBCPP_TEMPLATE_VIS` marks the vtable and typeinfo with default visibility. The class won't have a vtable since it doesn't have any virtual functions. Typeinfo visibility should only matter if you're throwing types across dynamic library boundaries, but since `__string_base_common` is an internal type, that shouldn't be an issue anyway. @EricWF can weigh in on that when he sees this though.


https://reviews.llvm.org/D35326