[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions
David Blaikie via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 4 14:16:50 PDT 2022
dblaikie added a comment.
In D119051#3747673 <https://reviews.llvm.org/D119051#3747673>, @rnk wrote:
> In D119051#3747201 <https://reviews.llvm.org/D119051#3747201>, @dblaikie wrote:
>
>> 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?
>
> Sounds good to me, I think you did the hard work of verifying, thanks for that. :)
>
> Anyway, removing this isPOD usage should be it's own patch, with tests. We should already have test coverage for aarch64 that can be reused. @ayzhao, can you help David with this? This is not C++20-related, but it is clang frontend related.
Popping the stack, the use of AST's isPOD in Microsoft's ABI has been removed in favor of more nuanced custom implementation in the Microsoft ABI handling that fixed a few bugs along the way (D133817 <https://reviews.llvm.org/D133817>, D134688 <https://reviews.llvm.org/D134688>).
I've rebased this patch and addressed a test failure I hadn't spotted before, in `clang/test/AST/conditionally-trivial-smfs.cpp` - added some details to the patch description.
And now that the Microsoft ABI doesn't depend on the AST isPOD property at all, I've removed the previously proposed new test `clang/test/CodeGenCXX/return-abi.cpp` that was testing the Microsoft return ABI.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119051/new/
https://reviews.llvm.org/D119051
More information about the cfe-commits
mailing list