[libcxx-commits] [PATCH] D146190: Fix EBO on std::optional and std::variant when targeting the MSVC ABI
Mark de Wever via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Mar 21 12:27:30 PDT 2023
Mordante added a comment.
Since @EricWF feels this is fine for Windows I don't have objections either. However I really feel we need to document this better.
================
Comment at: libcxx/include/optional:641
template <class _Tp>
-class optional
+class _LIBCPP_DECLSPEC_EMPTY_BASES optional
: private __optional_move_assign_base<_Tp>
----------------
davidben wrote:
> EricWF wrote:
> > Mordante wrote:
> > > Why not guard this attribute with a `_LIBCPP_ABI_FOO_FLAG` instead of breaking the ABI unconditionally?
> > Because that would prevent most users from ever getting value from it.
> >
> > If we don't have any libc++ users who care about ABI compatibility with MSVC, then we should do this unconditionally.
> Unconditional is also my preference, but I'll do whatever the project prefers. Currently assuming that's to leave it at is. (What's the process for driving that question to a conclusion?)
When that is the rationale for forcing an ABI break on all Windows users, then let's document that in the commit message.
I think it would be good to add this rationale to the `index.rst`, since this is against the normal libc++ policy. I really want to avoid people thinking an unstable ABI is acceptable on all platforms.
================
Comment at: libcxx/test/libcxx/utilities/variant/variant.variant/variant_size.pass.cpp:81
+ // Test that std::variant achieves the expected size. See https://github.com/llvm/llvm-project/issues/61095.
+ static_assert(sizeof(std::variant<char, char, char>) == sizeof(type_with_index<char>), "");
+ static_assert(sizeof(std::variant<int, int, int>) == sizeof(type_with_index<int>), "");
----------------
This is not needed in C++17, the same for the static_asserts below.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146190/new/
https://reviews.llvm.org/D146190
More information about the libcxx-commits
mailing list