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

Thu Aug 11 16:20:51 PDT 2022

dblaikie added a comment.

I guess the other packed behavior/ABI checking 277123376ce08c98b07c154bf83e4092a5d4d3c6 <https://reviews.llvm.org/rG277123376ce08c98b07c154bf83e4092a5d4d3c6>

In D119051#3715939 <https://reviews.llvm.org/D119051#3715939>, @aaron.ballman wrote:

> In D119051#3714645 <https://reviews.llvm.org/D119051#3714645>, @dblaikie wrote:
>
>> Realized maybe we don't need a separate driver flag for this at all, and rely only on the abi-compat flag? That seems to be how (at least some) other ABI compat changes have been handled, looking at other uses of `ClangABI` enum values.
>
> Agreed, I think this is the better approach.
>
>> There could be more testing than only the indirect result of the packing problem that first inspired this patch. Any suggestions on what might be the most direct way to test whether the type's been considered pod in this sense?
>
> 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

So I could test this other ways that actually impact layout - like whether things can be packed in tail padding (can pack in tail padding for non-pod, right?). Or we could ast dump and inspect the property directly? Maybe there are some other options?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051