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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
