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

Wed Oct 5 15:55:38 PDT 2022

rnk added inline comments.

================
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:
----------------
dblaikie wrote:
> 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?
My main concern is keeping complex target-specific conditions out of DeclCXX to improve readability. Any way to achieve that sounds good to me.

I think I'd lean towards passing LangOpts as a parameter. After that, storing `DefaultedSMFArePOD` on TargetInfo as a bool sounds good. You can set the field in `TargetInfo::adjust` to read LangOpts, and then override that in PS & Darwin specific ::adjust method overrides.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051