[PATCH] D52405: [libc++] Use exclude_from_explicit_instantiation instead of always_inline

Louis Dionne via Phabricator reviews at reviews.llvm.org
Wed Oct 3 18:03:16 PDT 2018


ldionne added a comment.

In https://reviews.llvm.org/D52405#1254572, @rsmith wrote:

> In https://reviews.llvm.org/D52405#1254544, @ldionne wrote:
>
> > In https://reviews.llvm.org/D52405#1254526, @rsmith wrote:
> >
> > > Makes sense to me. Is there some way you can write a test for this? (Maybe applying `nm` to a linked program?)
> >
> >
> > What I had in mind is that we should have a build bot that runs the test suite with/without `_LIBCPP_HIDE_FROM_ABI_PER_TU`. If all the current libc++ tests link, this seems to give us the most confidence that nothing is broken. Did you have something more specific in mind that you're concerned about? If so, I'm happy to write a test for it, I'd just like to understand what that is.
>
>
> I'm happy with any testing that works :)


Ok. Well, if I remove `__always_inline__` without using `exclude_from_explicit_instantiation`, I get many link errors in the libc++ test suite. So I'm confident that the fact that libc++ isn't broken is already tested, at least to some extent. Also, the well-functioning of the attribute itself is tested in the Clang patch.

Of course, it's possible that some unexpected thing is going to show up when I merge this in, as is always the case, but I don't easily see what tests I could add to reduce this risk at the moment.


Repository:
  rCXX libc++

https://reviews.llvm.org/D52405





More information about the libcxx-commits mailing list