[libcxx-commits] [PATCH] D62618: [libcxx] Make std::tuple<> trivially constructible

[Louis Dionne via Phabricator via libcxx-commits]

ldionne marked an inline comment as done.
ldionne added a comment.

In D62618#1521964 <https://reviews.llvm.org/D62618#1521964>, @EricWF wrote:

> Changing the triviality of a type can be ABI breaking because it will change the calling conventions the compiler uses.
>  We have hacks in pair to keep it non-trivial on some platforms for this reason.
>
> I'm not sure if this case triggers that change (it may only be when you change `is_trivially_copyable`). But we need to double check.


I was going to close PR41714 as "Can't fix, this is an ABI break", but then I checked the Itanium C++ ABI and it did not mention default constructors. Here <https://itanium-cxx-abi.github.io/cxx-abi/abi.html#calls>, it says:

> *non-trivial for the purposes of calls*
>  A type is considered non-trivial for the purposes of calls if:
> 
> - it has a non-trivial copy constructor, move constructor, or destructor, or
> - all of its copy and move constructors are deleted.
> 
>   This definition, as applied to class types, is intended to be the complement of the definition in [class.temporary]p3 of types for which an extra temporary is allowed when passing or returning a type. A type which is trivial for the purposes of the ABI will be passed and returned according to the rules of the underlying C ABI, e.g. in registers; often this has the effect of performing a trivial copy of the type.

I'd welcome a double-check, but I think this isn't ABI breaking. Otherwise the resolution is evident, y'all know me :-).



================
Comment at: libcxx/include/tuple:910
     _LIBCPP_INLINE_VISIBILITY
-    _LIBCPP_CONSTEXPR tuple() _NOEXCEPT {}
+    _LIBCPP_CONSTEXPR tuple() _NOEXCEPT _LIBCPP_DEFAULT
     template <class _Alloc>
----------------
EricWF wrote:
> This is C++11 and up so you can just write `= default`.
I thought this was trying to work in C++03 -- isn't that why we're using `_NOEXCEPT` and `_LIBCPP_CONSTEXPR`??


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62618