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

[David Blaikie via Phabricator via cfe-commits]

dblaikie 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:
----------------
rnk wrote:
> 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.
Fair enough - tried this out with a LongOpts parameter (I guess `getCallingConvKind` passes in just the boolean about whether it's the right ABI compatibility version - requiring the caller to check, but that seems a bit weird/specific? So I prefer passing in the whole LangOpts here and querying the right property inside TargetInfo)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051