[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

Fri Aug 12 11:33:37 PDT 2022

aaron.ballman added a comment.

> In D119051#3715939 <https://reviews.llvm.org/D119051#3715939>, @aaron.ballman wrote:
>
>> In D119051#3714645 <https://reviews.llvm.org/D119051#3714645>, @dblaikie wrote:
>>
>>> 
>>
>> I would have thought use of `__is_pod` would tell us, but I'm not seeing the behavior described in the test case when using that: https://godbolt.org/z/1vr3MK4KW Oddly, it seems that `QualType::isCXX11PODType()` doesn't look at `PlainOldData` at all! What is your expectation as to how the type trait should be behaving?
>
> Oh, yeah, seems @rsmith and I discussed this naming/expectations issue a bit over here previously: https://reviews.llvm.org/D117616#inline-1132622

Ah, thank you for that!

In D119051#3717934 <https://reviews.llvm.org/D119051#3717934>, @dblaikie wrote:

> I guess the other way to test for pod-for-purposes-of-ABI is IRgen. Looks like MSVC isn't observable based on sizeof or alignof on these issues (the previous godbolt shows MSVC's answer to alignment for the packed and size for the trailing packing don't change based on any of the variations (pod, non-pod, pod-with-defaulted-special-members)) - but should be observable based on the returning ABI.
>
> Ah, here we go: https://godbolt.org/z/sd88zTjPP
>
> So MSVC does consider the defaulted special member as still a valid pod-for-purposes-of-ABI and clang is incorrect/not ABI compatible with this. So MSVC does want this fix.

Yeah this strikes me as the right kind of test to add for testing the ABI (codegen cares more about ABI than Sema, broadly speaking).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051