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

[David Blaikie via Phabricator via cfe-commits]

dblaikie marked an inline comment as done.
dblaikie added a comment.

In D119051#3724121 <https://reviews.llvm.org/D119051#3724121>, @rnk wrote:

> Regarding the usage of isPOD in the MSVC ABI code, we should have a routine similar to `isTrivialForAArch64MSVC` that works for the other MSVC architectures that we support (x86, x64, and arm32). I don't think we should try to rely on definition data bits. Clang is almost always tracking something slightly different than what MSVC tracks, so we should write down MSVC's logic once and keep it separate from Clang's notion of POD.
>
> I think you can work out MSVC's behavior with godbolt, but if you need a hand, I volunteer @hans to take a stab at refactoring `MicrosoftCXXABI::classifyReturnType`. I remember making lots of comments to try to simplify the code during review, but everyone is always wary of scope creep.

Well, my first attempt at this - essentially using `isTrivialForAArch64MSVC` for everything, including x86, doesn't fail any Clang tests...

Do we still have those various fuzzers Majnemer setup? I'd guess we'd need to fuzz around here again to discover what the intended behavior is?

Hmm, let's compare the things that make `PlainOldData` `false`...

1. has base classes
2. any virtual member function
3. non-implicit ctor (so even user-declared-but-defaulted ctor)
4. non-implicit member function (so even user-declared-but-defaulted assignment operators or dtor)
5. any non-public field member
6. some ObjC thing (non-trivial ObjC lifetime)
7. any field that is of C++98 POD type... (mostly a recursive definition, when it comes to record types - but then there's some rules about incomplete array types, which couldn't be field types anyway, and some other cases)
8. non-static data member initializers

Let's compare that with `isTrivialForAArch64MSVC`...

1. checked (second)
2. checked (third) - well, it checks "is polymorphic" rather than virtual functions, so does that change things if you've got virtual bases but no virtual functions, can you have that?
3. only user-provided ctors, so it can have non-implicit (user-declared) but defaulted ctors
4. specifically checking non-trivial copy assignment (no restriction on move assignment and it could have any number of other member functions, I think?)
5. checked (first)
6. nope/not relevant?
7. Presumably tested by "hasNonTrivialDestructor" and "hasNonTrivialCopyAssignment"?
8. would a non-static data member initializer make the ctor user-provided? Presumably not, so I don't think this property is checked at all, which seems fair - how you pass something shouldn't be changed by how its constructed

I'll test the cases where these things differ with the godbolt-like testing earlier...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051