[PATCH] D24599: Add 'inline' but not _LIBCPP_INLINE_VISIBILITY to basic_string's destructor

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 15 12:10:00 PDT 2016


EricWF added a comment.

In https://reviews.llvm.org/D24599#543849, @mclow.lists wrote:

> Any reason we shouldn't just revert r280944, wait for the LLVM bug to be fixed, and then re-apply it?


I would like to put some time between fixing the Clang bug and re-introducing the reproducer into libc++.
Like it would be nice if 3.9 + libc++ still self hosted. I already reverted r280944 and I think we should put it
back eventually, but maybe not right after it's fixed.

In https://reviews.llvm.org/D24599#543917, @hiraditya wrote:

> @EricWF, since inline is only a hint, the compiler would not inline in many cases, it might give the inliner a little bit of push to inline. When we were working on this patch, adding inline wasn't enough and hence we added the _LIBCPP_INLINE_VISIBILITY flag. The compiler crash seems to be in the Verifier which does not allow aliases to available_externally functions.


I'm aware. As I mentioned in the summary Clang only listens to `inline` at -O2 or greater. However without `inline` it won't even get inlined then. This is more of a bandage than your complete solution.


https://reviews.llvm.org/D24599





More information about the cfe-commits mailing list