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

Wed Oct 5 15:17:54 PDT 2022

dblaikie added a comment.

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

> Relatedly, if we put this POD behavior query on TargetInfo, I wonder if it makes sense to move the tail padding predicate from TargetCXXABI to TargetInfo:
> https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/TargetCXXABI.h#L282
>
> The switch there is basically a switch over architectures.

Hmm, looked into this - but isn't most of `TargetCXXABI` a switch over CXXABIs, which are almost architectures/operating systems? Why is tail padding predicate different than the other tests in TargetCXXABI?

But I took a look at changing this in D135326 <https://reviews.llvm.org/D135326> in case you want to check if this is what you had in mind - I'm not entirely sure it's better. The OS seems not be quite 1:1 with the CXXABI groupings we have in TargetCXXABI... so one end result could just be calling back into the ABI to get the CXXABI kind and then returning based on that, which doesn't seem to be better...

================
Comment at: clang/lib/AST/DeclCXX.cpp:774-775
+        if ((!Constructor->isDeleted() && !Constructor->isDefaulted()) ||
+            (getLangOpts().getClangABICompat() <=
+             LangOptions::ClangABI::Ver15 || Target.isPS() || Target.isOSDarwin())) {
+          // C++ [class]p4:
----------------
rnk wrote:
> I think this ought to be factored into a TargetInfo method, so we can share the logic here and below somehow. Compare this for example with `TargetInfo::getCallingConvKind`, which has a similar purpose.
Seems plausible - though the version is stored in LangOpts, which isn't currently plumbed through into TargetInfo - should it be plumbed through, or is there some layering thing there where targets shouldn't depend on lang opts?

Looks like it'd mostly involve passing LangOpts down here: https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/CompilerInstance.cpp#L107 and plumbing it through all the `TargetInfo` ctors, possibly either storing `LangOpts&` in the `TargetInfo`, or computing the `DefaultedSMFArePOD` property in the ctor and storing that as a `bool` member in `TargetInfo` to return from some query function to be added to that hierarchy.

Or I guess like `getOSDefines` have `getDefaultedSMFArePOD` takes `LangOptions` as a parameter?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051