[libcxx-commits] [PATCH] D146190: Fix EBO on std::optional and std::variant when targeting the MSVC ABI

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.

Based on the discussion we had in D146190 <https://reviews.llvm.org/D146190> where we agreed that the ABI on MSVC was not to be considered stable, I think this patch does the right thing. There are a few things I think we can improve in the patch, but the overall goal sounds good to me.



================
Comment at: libcxx/test/libcxx/utilities/optional/optional.object/optional_size.pass.cpp:24
+int main(int, char**) {
+  // Test that std::optional achieves the expected size. See https://github.com/llvm/llvm-project/issues/61095.
+  static_assert(sizeof(std::optional<char>) == sizeof(type_with_bool<char>));
----------------



================
Comment at: libcxx/test/libcxx/utilities/variant/variant.variant/variant_size.pass.cpp:67
+#ifdef _LIBCPP_ABI_VARIANT_INDEX_TYPE_OPTIMIZATION
+  unsigned char index;
+#else
----------------
Can't we always use `unsigned char`? It would be nice to avoid checking an internal macro like `_LIBCPP_ABI_VARIANT_INDEX_TYPE_OPTIMIZATION` from the test suite if we can.


================
Comment at: libcxx/test/libcxx/utilities/variant/variant.variant/variant_size.pass.cpp:80
 
+  // 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>));
----------------



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

https://reviews.llvm.org/D146190