libc++ std::variant adds two words of overhead, rather than one, on Windows

[davidben]

I don't have an easy standalone reproducer, mostly because I don't see clang-cl on godbolt or anything, but here's a static_assert in Chromium:
https://chromium-review.googlesource.com/c/chromium/src/+/4294508
https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket/8787813272405926289/+/u/compile__with_patch_/stdout

While absl::variant<size_t, char> is 16 bytes as expected, std::variant<size_t, char>, on just Windows is 24 bytes.

[mordante]

Thanks for the report. I'm quite sure we have already fixed this issue. However due to ABI stability it's not enabled by default.
https://github.com/llvm/llvm-project/blob/main/libcxx/include/variant#L354
In ABI v1 we use an unsigned as index, in ABI v2 this variant would use an unsigned char as index. The exact sizes will depend on the size and alignment of size_t and unsigned.

I'm closing this issue, feel free to reopen it if you have more information.

[davidben]

I don't have permission to reopen it, but I don't think that solves it. We're already using ABI v2:
https://source.chromium.org/chromium/chromium/src/+/main:buildtools/third_party/libc++/__config_site;drc=0f58c67ce7efdf9bab8fd40f32193cfa8d35ec31;l=15

Also that fix seems to be about using uint8_t when there are fewer indices. In the case of std::variant<size_t, char>, alignment forces the index to have at least 8 byte overhead anyway. The problem is that, on Windows, we're getting 16-byte overhead instead of 8. I assume this is something to do with some Windows ABI differences that are causing the libc++ implementation to pick up an extra word of overhead somewhere.

[mordante]

Thanks for the additional information! With ABIv2 this indeed looks like a bug.

[davidben]

@jdoerrie points out there's a patch in https://reviews.llvm.org/D120064 to fix this. Looks like it needs test and a note that the MSVC ABI (the only one affected) is not stable. I don't have cycles right now, but may try to dust it off later if no one else has beaten me to it by then.

From that patch, it seems std::optional is also affected.

[davidben]

I don't have cycles right now, but may try to dust it off later if no one else has beaten me to it by then.

https://reviews.llvm.org/D146190