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

[Joe Loser via Phabricator via libcxx-commits]

jloser added inline comments.


================
Comment at: libcxx/include/ios:698
 template <class _CharT, class _Traits>
-basic_ios<_CharT, _Traits>::~basic_ios()
+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!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126738