[libcxx-commits] [PATCH] D126738: [libc++][NFC] Mark the definition of ~basic_ios::basic_ios() as inline
Nikolas Klauser via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Jun 1 01:07:36 PDT 2022
philnik added inline comments.
Comment at: libcxx/include/ios:698
template <class _CharT, class _Traits>
+inline basic_ios<_CharT, _Traits>::~basic_ios()
> jloser wrote:
> > ayzhao wrote:
> > > I'm not sure if I should specify `_LIBCPP_INLINE_VISIBILITY` here - does anyone have any ideas?
> > Should we use `inline _LIBCPP_INLINE_VISIBILITY` instead like the neighboring code currently does?
> So, `_LIBCPP_INLINE_VISIBILITY` is a historical predecessor to `_LIBCPP_HIDE_FROM_ABI`. In newer code, we've been updating to `_LIBCPP_HIDE_FROM_ABI`.
> With that being said, we definitely want `inline` there and also either `_LIBCPP_HIDE_FROM_ABI` or `_LIBCPP_INLINE_VISIBILITY`. There's an argument to be made for both. Using `_LIBCPP_HIDE_FROM_ABI` is the "new de-facto standard", whereas the latter is locally consistent in this neighborhood of code. I don't feel strongly.
> It's a bit surprising this destructor wasn't hidden from being an exported symbol in the ABI!
I think nobody would have a problem if we just remove the out-of-line definition and change line 642 to `_LIBCPP_HIDE_FROM_ABI virtual ~basic_ios() = default;`. I //think// it should be fine to make it `= default`, because all copy- and move constructors are deleted.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits