[libcxx-commits] [PATCH] D115275: [libc++] Remove _LIBCPP_DEFAULT

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone accepted this revision.
Quuxplusone added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/include/__config:840-844
-#ifdef _LIBCPP_CXX03_LANG
-#  define _LIBCPP_DEFAULT {}
-#else
-#  define _LIBCPP_DEFAULT = default;
-#endif
----------------
philnik wrote:
> Quuxplusone wrote:
> > I have looked at this in the past and decided not to mess with it, because it changes our ABI in C++03 mode. @ldionne, could you take another look with ABI in mind and just confirm whether this still LGTY?
> > 
> > Remember `=default`ed members can be trivial, but `{}`'ed members are never trivial, for purposes of ABI.
> Isn't `_LIBCPP_HIDE_FROM_ABI` supposed to mitiagte that problem?
No, in that we're talking about a property of the type (whether it's trivially foo-constructible), not a property of the member function per se.

However, I'm retracting my concern, because I just looked for myself: `_LIBCPP_DEFAULT` is currently used in only the places touched in this PR, and:
- `std::allocator` already does crazy things with inheritance to get the right trivially-default-constructibility in all modes, so it's not affected in C++03 mode by this patch AFAICT
- `std::atomic<T>` inherits from `__cxx_atomic_base_impl` which is non-trivial in C++03 mode, so it remains non-trivial in C++03 mode even after this patch
- `std::error_category` is polymorphic, so it's never trivially default constructible in any mode

I now say: Ship it! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115275