[libcxx-commits] [PATCH] D63744: In the libc++ unstable ABI, use [[no_unique_address]] instead of __compressed_pair when available.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D63744#2369798 <https://reviews.llvm.org/D63744#2369798>, @jyknight wrote:

> In D63744#2369637 <https://reviews.llvm.org/D63744#2369637>, @ldionne wrote:
>
>> If we assume that the compiler supports `[[no_unique_address]]`, is this an ABI break? Does `no_unique_address` result in a different layout than the `__compressed_pair`?
>
> Yes, this creates an incompatible memory layout. I don't think there's any way around that, while getting the benefits of removing the intermediate object.
>
> [...]
> As soon as you move the fields out of the intermediate object, you will lose some padding, and thus the layout will be different [...]

Ah, right, of course. In that case, though, we could do something like:

  class Foo {
    double a1;
    char a2;
  #if defined(_LIBCPP_ABI_STABLE)
    char __abi_stable_padding_blablabla[7];
  #endif
    short b1;
    char b2;
  #if defined(_LIBCPP_ABI_STABLE)
    char __abi_stable_padding2_blablabla[1];
  #endif
  };

And that would preserve the same layout. Or am I forgetting something else here?

In that case, we could have a utility that creates the right padding for translating from `__compressed_pair<T, U>` to a sequence of `[[no_unique_address]] T; [[no_unique_address]] U;`. The usage would be something along the lines of:

  class Foo {
    double a1;
    char a2;
  #if defined(_LIBCPP_ABI_STABLE)
    [[no_unique_address]] __old_compressed_pair_ABI_padding<double, char> __padding1;
  #endif
    short b1;
    char b2;
  #if defined(_LIBCPP_ABI_STABLE)
    [[no_unique_address]] __old_compressed_pair_ABI_padding<short, char> __padding2;
  #endif
  };

This would expand to the right amount of padding for preserving ABI, and it would make sure we don't have to hardcode number of bytes in the code (which is error prone because of platform differences).

And this would neatly preserve ABI while removing 90% of the noise in this patch. Only the member declarations would contain a bit of noise aimed specifically at preserving ABI. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63744