[PATCH] D50652: [libcxx] By default, do not use internal_linkage to hide symbols from the ABI

[Hans Wennborg via Phabricator via cfe-commits]

hans accepted this revision.
hans added a comment.

>>> Oh, or could we do
>>> 
>>>   -D_LIBCPP_HIDE_FROM_ABI=
>>> 
>>> 
>>> and just get regular odr linkage for these functions?
>> 
>> No, you do need to use `_LIBCPP_HIDDEN _LIBCPP_ALWAYS_INLINE` because of the issue described in http://lists.llvm.org/pipermail/llvm-dev/2018-July/124549.html. But yeah, Chromium could use this workaround.
> 
> Actually, scratch that, it does work. One can either use `-D_LIBCPP_HIDE_FROM_ABI=_LIBCPP_HIDDEN _LIBCPP_ALWAYS_INLINE` to restore the old behavior, or `-D_LIBCPP_HIDE_FROM_ABI=` to get odr linkage.

I tried `-D_LIBCPP_HIDE_FROM_ABI=` but got lots of link errors (see https://bugs.chromium.org/p/chromium/issues/detail?id=872926#c21).

`-D_LIBCPP_HIDE_FROM_ABI=_LIBCPP_HIDDEN _LIBCPP_ALWAYS_INLINE` works nicely though.

I also tried applying your patch and verified that works for Chromium. If I understand correctly, without setting LIBCXX_HIDE_FROM_ABI_PER_TU_BY_DEFAULT, it restores the old behaviour of force inlining.

I think this makes sense for Chromium since it allows us to build without any special flags until we can get ODR linkage, and I think it also makes sense for the 7.0 branch to prevent regressing binary sizes for users of the release.

lgtm


Repository:
  rCXX libc++

https://reviews.llvm.org/D50652