[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