[libcxx-commits] [PATCH] D126738: [libc++][NFC] Inline the definition of ~basic_ios()

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D126738#3550536 <https://reviews.llvm.org/D126738#3550536>, @philnik wrote:

> LGTM assuming CI passes, but please wait for a second #libc <https://reviews.llvm.org/tag/libc/> reviewer before landing the patch.

I'm pretty sure CI is going to fail during `check-cxx-abilist`.

As-is, this patch is an ABI break for any user of `iostream` because it removes `std::__1::basic_ios<wchar_t, std::__1::char_traits<wchar_t> >::~basic_ios()` and `std::__1::basic_ios<char, std::__1::char_traits<char> >::~basic_ios()` from the dylib. Furthermore, I believe there was probably an intent of this destructor acting as a key function so that the vtable and RTTI of `basic_ios<char|wchar_t>` ends up in the dylib, and in the dylib only. I'm not 100% sure about that, but that's the most likely explanation for this strange definition in the first place.

After reading https://crbug.com/1327706#c13, it seems like the issue comes up when enabling libc++'s debug mode, right? If so, then I think this must be tackled via D122941 <https://reviews.llvm.org/D122941> or something like it, instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126738/new/

https://reviews.llvm.org/D126738