ldionne added a comment.

In https://reviews.llvm.org/D35388#1167315, @thomasanderson wrote:

> In https://reviews.llvm.org/D35388#1167305, @ldionne wrote:
>
> > ABI-wise, I think this change is OK. Indeed, if `__type_visibility__` is 
> > not supported, we're already marking the base template as 
> > `__visibility__("default")`, so marking the extern template declaration 
> > with `__visibility__("default")` is not a problem. If `__type_visibility__` 
> > is supported, then there's no change in behavior. So I think there is no 
> > change in behavior either way, and I'm actually wondering why 
> > @thomasanderson wants this change in if the behavior is always the same. 
> > Perhaps I am missing something?
>
>
> It's been a while, but IIRC this fixes the Chromium build with gcc and 
> libc++, since std::vector_base_common<true>::__throw_length_error() wasn't 
> getting exported otherwise.


Ah, well of course! That's because I assumed that 
`_LIBCPP_EXTERN_TEMPLATE_TYPE_VIS` was always used on extern template 
declarations where the base template had been declared with 
`_LIBCPP_TEMPLATE_VIS`, which isn't true for `__vector_base_common`.

> See the comment in the Chromium source code:
>  
> https://cs.chromium.org/chromium/src/buildtools/third_party/libc%2B%2B/BUILD.gn?rcl=0dd5c6f980d22be96b728155249df2da355989d9&l=88
> 
> And also this CL https://reviews.llvm.org/D35326

I would think that both this change and https://reviews.llvm.org/D35326 are 
good.


https://reviews.llvm.org/D35388



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to