[libcxx-commits] [PATCH] D122598: [libcxx] avoid using anonymous struct with base classes (fixes gcc-12)

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

So I was wondering how to make sure this change is good, and wasn't too sure. It turns out that we don't have any ABI compatibility testing for this code path. I'm adding that in D123081 <https://reviews.llvm.org/D123081>, so I'd like for D123081 <https://reviews.llvm.org/D123081> to land before we land this. Once we have backdeployment testing running on arm64, we can try messing up the padding here in a subtle way, confirm that it breaks our tests, and then move forward with this patch with confidence (and satisfaction knowing our tests really have our back).

This looks correct to me, but still requesting changes so it shows up in the review queue.



================
Comment at: libcxx/include/string:690
         value_type __data_[__min_cap];
-        struct
-            : __padding<value_type>
-        {
-            unsigned char __size_;
-        };
+        char __padding[sizeof(value_type) - 1];
+        unsigned char __size_;
----------------
This should be `unsigned char __padding[...]`. It doesn't change anything, but it makes it clearer that we're padding the `unsigned char __size_` member that follows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122598