[libcxx-commits] [PATCH] D126738: [libc++][NFC] Inline the definition of ~basic_ios()
Louis Dionne via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Jun 1 10:54:36 PDT 2022
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.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits