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

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Each godbolt link shows  MSVC 19.latest and clang, both in x86 and aarch64, with a reference pod and non-pod example at the start and end, and the variable test in the middle to see whether the codegen matches the pod or non-pod baselines above and below.

> 1. checked (second): has base classes

https://godbolt.org/z/d76fEGP1M - all agreed

> 2. checked (third): virtual functions/polymorphic

Oh, I guess if you can't have base classes then testing 'is polymorphic' is equivalent to 'has virtual functions'

This one's slightly less obvious to verify due to the existence of a virtual function changing the codegen of the `f1` test functions so comparing it to the pod/non-pod reference  & maybe me think about how to better isolate the test - though I don't think it's possible to totally isolate it, adding a virtual functions adds a vptr that has to be copied ,etc and extra globals for the vtalbe itself (MSVC doesn't home vtables), etc.

So I've got this: https://godbolt.org/z/nGa4drG4h - but it looks like they all agree, despite the extra variations in the output

> 3. only user-provided ctors, so it can have non-implicit (user-declared) but defaulted ctors

User-defined ctor (`t1(int)` unrelated to any usage/copy operation, etc): https://godbolt.org/z/e79b1zeqr - everyone agrees
User-declared-but-not-user-defined ctor (`t1() = default`): Clang x86 is incorrect, believes it to be non-trivial - that's the original issue under discussion in this bug

> 4. specifically checking non-trivial copy assignment (no restriction on move assignment and it could have any number of other member functions, I think?)

Oh, everyone allows a non-trivial non-special member function, I misread the Clang code - those don't affect retro-POD either, so everyone agrees on that. https://godbolt.org/z/hnMfqjov7

Restricting this to Special Member Functions, defaulted copy constructor... https://godbolt.org/z/sGvxcEPjo Clang x86 is incorrect

> 5. checked (first)

Used this as the baseline - everyone agrees.

> 6. nope/not relevant?



-

> 7. Presumably tested by "hasNonTrivialDestructor" and "hasNonTrivialCopyAssignment"?

Having a field of a type with a protected member (property 5) - Clang x86 is incorrect: https://godbolt.org/z/GY48qxh3G
Having a field of a type with a non-trivial dtor - everyone agrees: https://godbolt.org/z/4dq9b43ac
Having a field of a type with a non-trivial copy-assignment - everyone agrees: https://godbolt.org/z/WEnKcz7oW

> 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

Non-static data member initializer: https://godbolt.org/z/Mb1PYhjrP - Clang x86 is incorrect

So... my conclusion is that Clang's AArch64 appears to be correct for x86  as well, and we should just rename the function and use it unconditionally, removing any use of Clang's AST POD property in the MSVC ABI handling?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051