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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.



>> 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?

Ah, yeah, my understanding is the POD we're talking about is basically "POD for the purposes of Itanium layout" - independent of various language definitions of POD.

Which got me searching around in the code for uses of the data - it's used in some warnings, but also in the MSVC layout ( https://github.com/llvm/llvm-project/blob/c8cf669f6025bdf0fc227f3ddbbf3523a5b32f0b/clang/lib/CodeGen/MicrosoftCXXABI.cpp#L1123 and it looks like MSVC proper uses the non-GCC-compatible choice here, so this patch shouldn't apply to MSVC compatibility mode... https://godbolt.org/z/Mvroof8n4 ).

I realized when removing the explicit flag I lost some of the overrides - for PS and Darwin. Since we're checking this property in two places and now it'll involve about 4 different checks (or should all these platforms imply the older ABI? At least that seems correct for PS and Darwin where their ABIs are defined by the older version of clang, but the MSVC case is weirder - since their ABI isn't defined by the older version of clang, and the ABI may need to change as we discover mismatches/mistakes in Clang's ABI implementation)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051