rsmith added inline comments.
================
Comment at: include/clang/AST/DeclCXX.h:1489-1491
+ bool shouldBeDestructedInCallee() const {
+ return data().CanPassInRegisters && hasNonTrivialDestructor();
+ }
----------------
This will return incorrect results on MSVC, where every class type should be
destroyed in the callee. Since this is only used by CodeGen, and only used in
combination with the MSVC "destroy right-to-left in callee" ABI setting, please
sink it to CodeGen.
================
Comment at: lib/CodeGen/CGCall.cpp:3518
+ CGM.getCXXABI().getRecordArgABI(RD) != CGCXXABI::RAA_Default) ||
+ TypeDestructedInCallee;
if (DestroyedInCallee)
----------------
This looks wrong (but the wrongness is pre-existing): in the weird MSVC x86_64
case where a small object of class type is passed in registers despite having a
non-trivial destructor, this will set `DestroyedInCallee` to false, meaning
that the parameter will be destroyed twice, in both the caller and callee.
And yep, that's exactly what both Clang and MSVC do:
https://godbolt.org/g/zjeQq6
If we fixed that, we could unconditionally `setExternallyDestructed()` here.
But let's leave that discussion for a separate patch :)
================
Comment at: lib/CodeGen/CGDecl.cpp:1822
+ bool IsScalar = hasScalarEvaluationKind(Ty);
+ if ((!IsScalar && !CurFuncIsThunk &&
+ getTarget().getCXXABI().areArgsDestroyedLeftToRightInCallee()) ||
----------------
Should these conditions not also apply to the `shouldBeDestructedInCallee`
case? We don't want to destroy `trivial_abi` parameters in thunks, only in the
eventual target function.
================
Comment at: lib/Sema/SemaDeclCXX.cpp:5923
+ M->setTrivialForCall(HasTrivialABI);
+ Record->finishedUserProvidedMethod(M);
+ }
----------------
This should have a different name if you're only going to call it for
user-provided copy ctors, move ctors, and dtors.
================
Comment at: lib/Sema/SemaDeclCXX.cpp:12108-12114
+ if (!IsTrivialForCall) {
+ if (ClassDecl->needsOverloadResolutionForCopyConstructor())
+ IsTrivialForCall =
+ SpecialMemberIsTrivial(CopyConstructor, CXXCopyConstructor, true);
+ else
+ IsTrivialForCall = ClassDecl->hasTrivialCopyConstructorForCall();
+ }
----------------
Nit: can you use the same form of `?:` expression as is used a few lines above
to make it obvious to a reader that this computation parallels that one?
https://reviews.llvm.org/D41039
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
