PR19668, PR23034: Fix handling of move constructors and deleted copy
constructors when deciding whether classes should be passed indirectly.
This fixes ABI differences between Clang and GCC:
* Previously, Clang ignored the move constructor when making this
determination. It now takes the move constructor into account, per
https://github.com/itanium-cxx-abi/cxx-abi/pull/17 (this change may
seem recent, but the ABI change was agreed on the Itanium C++ ABI
list a long time ago).
* Previously, Clang's behavior when the copy constructor was deleted
was unstable -- depending on whether the lazy declaration of the
copy constructor had been triggered, you might get different behavior.
We now eagerly declare the copy constructor whenever its deletedness
is unclear, and ignore deleted copy/move constructors when looking for
a trivial such constructor.
This also fixes an ABI difference between Clang and MSVC:
* If the copy constructor would be implicitly deleted (but has not been
lazily declared yet), for instance because the class has an rvalue
reference member, we would pass it directly. We now pass such a class
indirectly, matching MSVC.
Based on a patch by Vassil Vassilev, which was based on a patch by Bernd
Schmidt, which was based on a patch by Reid Kleckner!
This is a re-commit of r310401, which was reverted in r310464 due to ARM
failures (which should now be fixed).
llvm-svn: 310983
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 6e33b98..2c0bb11 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -956,12 +956,16 @@
ToData.HasUninitializedFields = FromData.HasUninitializedFields;
ToData.HasInheritedConstructor = FromData.HasInheritedConstructor;
ToData.HasInheritedAssignment = FromData.HasInheritedAssignment;
+ ToData.NeedOverloadResolutionForCopyConstructor
+ = FromData.NeedOverloadResolutionForCopyConstructor;
ToData.NeedOverloadResolutionForMoveConstructor
= FromData.NeedOverloadResolutionForMoveConstructor;
ToData.NeedOverloadResolutionForMoveAssignment
= FromData.NeedOverloadResolutionForMoveAssignment;
ToData.NeedOverloadResolutionForDestructor
= FromData.NeedOverloadResolutionForDestructor;
+ ToData.DefaultedCopyConstructorIsDeleted
+ = FromData.DefaultedCopyConstructorIsDeleted;
ToData.DefaultedMoveConstructorIsDeleted
= FromData.DefaultedMoveConstructorIsDeleted;
ToData.DefaultedMoveAssignmentIsDeleted
@@ -973,6 +977,7 @@
= FromData.HasConstexprNonCopyMoveConstructor;
ToData.HasDefaultedDefaultConstructor
= FromData.HasDefaultedDefaultConstructor;
+ ToData.CanPassInRegisters = FromData.CanPassInRegisters;
ToData.DefaultedDefaultConstructorIsConstexpr
= FromData.DefaultedDefaultConstructorIsConstexpr;
ToData.HasConstexprDefaultConstructor
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 1caceab..5782b7b 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -55,15 +55,18 @@
HasOnlyCMembers(true), HasInClassInitializer(false),
HasUninitializedReferenceMember(false), HasUninitializedFields(false),
HasInheritedConstructor(false), HasInheritedAssignment(false),
+ NeedOverloadResolutionForCopyConstructor(false),
NeedOverloadResolutionForMoveConstructor(false),
NeedOverloadResolutionForMoveAssignment(false),
NeedOverloadResolutionForDestructor(false),
+ DefaultedCopyConstructorIsDeleted(false),
DefaultedMoveConstructorIsDeleted(false),
DefaultedMoveAssignmentIsDeleted(false),
DefaultedDestructorIsDeleted(false), HasTrivialSpecialMembers(SMF_All),
DeclaredNonTrivialSpecialMembers(0), HasIrrelevantDestructor(true),
HasConstexprNonCopyMoveConstructor(false),
HasDefaultedDefaultConstructor(false),
+ CanPassInRegisters(true),
DefaultedDefaultConstructorIsConstexpr(true),
HasConstexprDefaultConstructor(false),
HasNonLiteralTypeFieldsOrBases(false), ComputedVisibleConversions(false),
@@ -352,8 +355,10 @@
setHasVolatileMember(true);
// Keep track of the presence of mutable fields.
- if (BaseClassDecl->hasMutableFields())
+ if (BaseClassDecl->hasMutableFields()) {
data().HasMutableFields = true;
+ data().NeedOverloadResolutionForCopyConstructor = true;
+ }
if (BaseClassDecl->hasUninitializedReferenceMember())
data().HasUninitializedReferenceMember = true;
@@ -406,6 +411,8 @@
// -- a direct or virtual base class B that cannot be copied/moved [...]
// -- a non-static data member of class type M (or array thereof)
// that cannot be copied or moved [...]
+ if (!Subobj->hasSimpleCopyConstructor())
+ data().NeedOverloadResolutionForCopyConstructor = true;
if (!Subobj->hasSimpleMoveConstructor())
data().NeedOverloadResolutionForMoveConstructor = true;
@@ -426,6 +433,7 @@
// -- any non-static data member has a type with a destructor
// that is deleted or inaccessible from the defaulted [ctor or dtor].
if (!Subobj->hasSimpleDestructor()) {
+ data().NeedOverloadResolutionForCopyConstructor = true;
data().NeedOverloadResolutionForMoveConstructor = true;
data().NeedOverloadResolutionForDestructor = true;
}
@@ -711,8 +719,10 @@
data().IsStandardLayout = false;
// Keep track of the presence of mutable fields.
- if (Field->isMutable())
+ if (Field->isMutable()) {
data().HasMutableFields = true;
+ data().NeedOverloadResolutionForCopyConstructor = true;
+ }
// C++11 [class.union]p8, DR1460:
// If X is a union, a non-static data member of X that is not an anonymous
@@ -756,6 +766,12 @@
// A standard-layout class is a class that:
// -- has no non-static data members of type [...] reference,
data().IsStandardLayout = false;
+
+ // C++1z [class.copy.ctor]p10:
+ // A defaulted copy constructor for a class X is defined as deleted if X has:
+ // -- a non-static data member of rvalue reference type
+ if (T->isRValueReferenceType())
+ data().DefaultedCopyConstructorIsDeleted = true;
}
if (!Field->hasInClassInitializer() && !Field->isMutable()) {
@@ -809,6 +825,10 @@
// We may need to perform overload resolution to determine whether a
// field can be moved if it's const or volatile qualified.
if (T.getCVRQualifiers() & (Qualifiers::Const | Qualifiers::Volatile)) {
+ // We need to care about 'const' for the copy constructor because an
+ // implicit copy constructor might be declared with a non-const
+ // parameter.
+ data().NeedOverloadResolutionForCopyConstructor = true;
data().NeedOverloadResolutionForMoveConstructor = true;
data().NeedOverloadResolutionForMoveAssignment = true;
}
@@ -819,6 +839,8 @@
// -- X is a union-like class that has a variant member with a
// non-trivial [corresponding special member]
if (isUnion()) {
+ if (FieldRec->hasNonTrivialCopyConstructor())
+ data().DefaultedCopyConstructorIsDeleted = true;
if (FieldRec->hasNonTrivialMoveConstructor())
data().DefaultedMoveConstructorIsDeleted = true;
if (FieldRec->hasNonTrivialMoveAssignment())
@@ -830,6 +852,8 @@
// For an anonymous union member, our overload resolution will perform
// overload resolution for its members.
if (Field->isAnonymousStructOrUnion()) {
+ data().NeedOverloadResolutionForCopyConstructor |=
+ FieldRec->data().NeedOverloadResolutionForCopyConstructor;
data().NeedOverloadResolutionForMoveConstructor |=
FieldRec->data().NeedOverloadResolutionForMoveConstructor;
data().NeedOverloadResolutionForMoveAssignment |=
@@ -915,8 +939,10 @@
}
// Keep track of the presence of mutable fields.
- if (FieldRec->hasMutableFields())
+ if (FieldRec->hasMutableFields()) {
data().HasMutableFields = true;
+ data().NeedOverloadResolutionForCopyConstructor = true;
+ }
// C++11 [class.copy]p13:
// If the implicitly-defined constructor would satisfy the
@@ -1450,7 +1476,7 @@
void CXXRecordDecl::completeDefinition(CXXFinalOverriderMap *FinalOverriders) {
RecordDecl::completeDefinition();
-
+
// If the class may be abstract (but hasn't been marked as such), check for
// any pure final overriders.
if (mayBeAbstract()) {