[PATCH] D49240: [libc++] Introduce _LIBCPP_HIDE_FROM_ABI to replace _LIBCPP_INLINE_VISIBILITY

[Louis Dionne via Phabricator via llvm-commits]

ldionne added a comment.

In https://reviews.llvm.org/D49240#1197149, @hans wrote:

> In https://reviews.llvm.org/D49240#1197052, @ldionne wrote:
>
> > In https://reviews.llvm.org/D49240#1196878, @hans wrote:
> >
> > > The reason we noticed this was that it caused a *50 GB* size increase of the build output on our buildbots, which was enough to cause infrastructure problems.
> > >
> > > This change was also committed shortly before the 7.0 branch, so it's part of the 7.0.0 release candidates.
> > >
> > > Should we back it out until a solution is found?
> >
> >
> > The thing is -- there's no solution without changing the guarantees that libc++ make. Today, libc++ guarantees that you can link TUs that were built with different versions of libc++ together. If we remove that guarantee, then we can use linkonce_odr and solve the problem that Chromium is having.
> >
> > Is that guarantee something that Chromium is relying upon?
>
>
> I'm not sure (thakis can probably answer better), but isn't it enough to link against some system libraries that might use libc++ for this to be true?


One would have to link statically against a system library that was statically linked against libc++ -- I don't think this happens, at least not on Darwin AFAICT. As soon as we cross a dylib boundary, we're safe because those symbols hidden from the ABI are not exported from the dylib. The only problem is when distributing static archives using different versions of libc++, where we don't want presumably ODR-equivalent symbols to be deduplicated (because they might not actually be ODR-equivalent).

> The previous solution, using inlining, while not very elegant didn't have this giant binary size problem, so maybe it was better?

Just to be clear, it's only about the number of symbols, not actual code size.

> What should we put in the release notes, that folks should expect significantly larger binaries using the 7.0.0 version?

If the current state is not acceptable, we should roll back the change and only put it in once we _also_ know how to allow ODR-deduplicating across TUs. We don't have a solution for that right now -- this was a follow-up step that was planned in the future since this one was semantics-changing. If the current state is acceptable, we can put in the release notes that significant symbol size increases should be expected using LLVM 7.0.0.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49240