v.g.vassilev updated this revision to Diff 108505.
v.g.vassilev added a comment.
Move the checks in Sema.
https://reviews.llvm.org/D35056
Files:
include/clang/AST/DeclCXX.h
lib/AST/ASTImporter.cpp
lib/AST/DeclCXX.cpp
lib/CodeGen/CGCXXABI.cpp
lib/CodeGen/ItaniumCXXABI.cpp
lib/Sema/SemaDecl.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriter.cpp
test/CodeGenCXX/uncopyable-args.cpp
Index: test/CodeGenCXX/uncopyable-args.cpp
===================================================================
--- test/CodeGenCXX/uncopyable-args.cpp
+++ test/CodeGenCXX/uncopyable-args.cpp
@@ -52,12 +52,11 @@
void bar() {
foo({});
}
-// FIXME: The copy ctor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN9move_ctor3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
+// CHECK-LABEL: define void @_ZN9move_ctor3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN9move_ctor3fooENS_1AE(%"struct.move_ctor::A"*)
// WIN64-LABEL: declare void @"\01?foo@move_ctor@@YAXUA@1@@Z"(%"struct.move_ctor::A"*)
}
@@ -73,12 +72,11 @@
void bar() {
foo({});
}
-// FIXME: The copy ctor is deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11all_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
+// CHECK-LABEL: define void @_ZN11all_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11all_deleted3fooENS_1AE(%"struct.all_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@all_deleted@@YAXUA@1@@Z"(%"struct.all_deleted::A"*)
}
@@ -93,12 +91,11 @@
void bar() {
foo({});
}
-// FIXME: The copy and move ctors are implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN18implicitly_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
+// CHECK-LABEL: define void @_ZN18implicitly_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN18implicitly_deleted3fooENS_1AE(%"struct.implicitly_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@implicitly_deleted@@YAXUA@1@@Z"(%"struct.implicitly_deleted::A"*)
}
@@ -113,12 +110,11 @@
void bar() {
foo({});
}
-// FIXME: The copy constructor is implicitly deleted.
-// CHECK-DISABLED-LABEL: define void @_ZN11one_deleted3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED-NOT: call
-// CHECK-DISABLED: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
+// CHECK-LABEL: define void @_ZN11one_deleted3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK-NOT: call
+// CHECK: call void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN11one_deleted3fooENS_1AE(%"struct.one_deleted::A"*)
// WIN64-LABEL: declare void @"\01?foo@one_deleted@@YAXUA@1@@Z"(%"struct.one_deleted::A"*)
}
@@ -195,12 +191,10 @@
void bar() {
foo({});
}
-// FIXME: This class has a non-trivial copy ctor and a trivial copy ctor. It's
-// not clear whether we should pass by address or in registers.
-// CHECK-DISABLED-LABEL: define void @_ZN14two_copy_ctors3barEv()
-// CHECK-DISABLED: call void @_Z{{.*}}C1Ev(
-// CHECK-DISABLED: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
-// CHECK-DISABLED-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
+// CHECK-LABEL: define void @_ZN14two_copy_ctors3barEv()
+// CHECK: call void @_Z{{.*}}C1Ev(
+// CHECK: call void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"* %{{.*}})
+// CHECK-LABEL: declare void @_ZN14two_copy_ctors3fooENS_1BE(%"struct.two_copy_ctors::B"*)
// WIN64-LABEL: declare void @"\01?foo@two_copy_ctors@@YAXUB@1@@Z"(%"struct.two_copy_ctors::B"*)
}
Index: lib/Serialization/ASTWriter.cpp
===================================================================
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -5885,6 +5885,7 @@
Record->push_back(Data.HasIrrelevantDestructor);
Record->push_back(Data.HasConstexprNonCopyMoveConstructor);
Record->push_back(Data.HasDefaultedDefaultConstructor);
+ Record->push_back(Data.HasNonDeletedCopyOrMoveConstructor);
Record->push_back(Data.DefaultedDefaultConstructorIsConstexpr);
Record->push_back(Data.HasConstexprDefaultConstructor);
Record->push_back(Data.HasNonLiteralTypeFieldsOrBases);
Index: lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -1570,6 +1570,7 @@
Data.HasIrrelevantDestructor = Record.readInt();
Data.HasConstexprNonCopyMoveConstructor = Record.readInt();
Data.HasDefaultedDefaultConstructor = Record.readInt();
+ Data.HasNonDeletedCopyOrMoveConstructor = Record.readInt();
Data.DefaultedDefaultConstructorIsConstexpr = Record.readInt();
Data.HasConstexprDefaultConstructor = Record.readInt();
Data.HasNonLiteralTypeFieldsOrBases = Record.readInt();
@@ -1708,6 +1709,7 @@
MATCH_FIELD(HasIrrelevantDestructor)
OR_FIELD(HasConstexprNonCopyMoveConstructor)
OR_FIELD(HasDefaultedDefaultConstructor)
+ MATCH_FIELD(HasNonDeletedCopyOrMoveConstructor)
MATCH_FIELD(DefaultedDefaultConstructorIsConstexpr)
OR_FIELD(HasConstexprDefaultConstructor)
MATCH_FIELD(HasNonLiteralTypeFieldsOrBases)
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -14812,6 +14812,48 @@
AllIvarDecls.push_back(Ivar);
}
+static bool HasNonDeletedCopyOrMoveCtor(CXXRecordDecl *CXXRD, Sema &SemaRef) {
+ // FIXME: Handle the cases in which CXXRD is dependent. We need to instantiate
+ // the class to figure out if the constructor should be deleted.
+ assert(!CXXRD->isDependentType() && "Not implemented yet");
+ if (CXXRD->needsImplicitCopyConstructor()) {
+ CXXConstructorDecl *CopyCtor = SemaRef.DeclareImplicitCopyConstructor(CXXRD);
+ if (SemaRef.ShouldDeleteSpecialMember(CopyCtor, Sema::CXXCopyConstructor))
+ SemaRef.SetDeclDeleted(CopyCtor, CXXRD->getLocation());
+ }
+
+ if (CXXRD->needsImplicitMoveConstructor()) {
+ CXXConstructorDecl *MoveCtor = SemaRef.DeclareImplicitMoveConstructor(CXXRD);
+ if (SemaRef.ShouldDeleteSpecialMember(MoveCtor, Sema::CXXMoveConstructor))
+ SemaRef.SetDeclDeleted(MoveCtor, CXXRD->getLocation());
+ }
+
+ bool CopyOrMoveDeleted = false;
+ for (const CXXConstructorDecl *CD : CXXRD->ctors()) {
+ if (CD->isMoveConstructor() || CD->isCopyConstructor()) {
+ if (!CD->isDeleted()) {
+ return true;
+ }
+ CopyOrMoveDeleted = true;
+ }
+ }
+
+ // If a move constructor or move assignment operator was declared, the
+ // default copy constructors are implicitly deleted, except in one case
+ // related to compatibility with MSVC pre-2015.
+ if (CXXRD->hasUserDeclaredMoveConstructor())
+ CopyOrMoveDeleted = true;
+ if (CXXRD->hasUserDeclaredMoveAssignment()) {
+ const LangOptions &opts = SemaRef.getASTContext().getLangOpts();
+ bool DeletesOnlyMatchingCopy =
+ opts.MSVCCompat && !opts.isCompatibleWithMSVC(LangOptions::MSVC2015);
+ if (!DeletesOnlyMatchingCopy)
+ CopyOrMoveDeleted = true;
+ }
+
+ return !CopyOrMoveDeleted;
+}
+
void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
ArrayRef<Decl *> Fields, SourceLocation LBrac,
SourceLocation RBrac, AttributeList *Attr) {
@@ -15090,7 +15132,12 @@
Record->setInvalidDecl();
}
}
- CXXRecord->completeDefinition(&FinalOverriders);
+
+
+
+ CXXRecord->completeDefinition(&FinalOverriders,
+ HasNonDeletedCopyOrMoveCtor(CXXRecord,
+ *this));
Completed = true;
}
}
Index: lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -63,11 +63,8 @@
bool classifyReturnType(CGFunctionInfo &FI) const override;
RecordArgABI getRecordArgABI(const CXXRecordDecl *RD) const override {
- // Structures with either a non-trivial destructor or a non-trivial
- // copy constructor are always indirect.
- // FIXME: Use canCopyArgument() when it is fixed to handle lazily declared
- // special members.
- if (RD->hasNonTrivialDestructor() || RD->hasNonTrivialCopyConstructor())
+ // If C++ prohibits us from making a copy, pass by address.
+ if (!canCopyArgument(RD))
return RAA_Indirect;
return RAA_Default;
}
@@ -998,10 +995,8 @@
if (!RD)
return false;
- // Return indirectly if we have a non-trivial copy ctor or non-trivial dtor.
- // FIXME: Use canCopyArgument() when it is fixed to handle lazily declared
- // special members.
- if (RD->hasNonTrivialDestructor() || RD->hasNonTrivialCopyConstructor()) {
+ // If C++ prohibits us from making a copy, return by address.
+ if (!canCopyArgument(RD)) {
auto Align = CGM.getContext().getTypeAlignInChars(FI.getReturnType());
FI.getReturnInfo() = ABIArgInfo::getIndirect(Align, /*ByVal=*/false);
return true;
Index: lib/CodeGen/CGCXXABI.cpp
===================================================================
--- lib/CodeGen/CGCXXABI.cpp
+++ lib/CodeGen/CGCXXABI.cpp
@@ -30,6 +30,9 @@
}
bool CGCXXABI::canCopyArgument(const CXXRecordDecl *RD) const {
+ // See also Sema::ShouldDeleteSpecialMember. These two functions
+ // should be kept consistent.
+
// If RD has a non-trivial move or copy constructor, we cannot copy the
// argument.
if (RD->hasNonTrivialCopyConstructor() || RD->hasNonTrivialMoveConstructor())
@@ -41,27 +44,7 @@
// We can only copy the argument if there exists at least one trivial,
// non-deleted copy or move constructor.
- // FIXME: This assumes that all lazily declared copy and move constructors are
- // not deleted. This assumption might not be true in some corner cases.
- bool CopyDeleted = false;
- bool MoveDeleted = false;
- for (const CXXConstructorDecl *CD : RD->ctors()) {
- if (CD->isCopyConstructor() || CD->isMoveConstructor()) {
- assert(CD->isTrivial());
- // We had at least one undeleted trivial copy or move ctor. Return
- // directly.
- if (!CD->isDeleted())
- return true;
- if (CD->isCopyConstructor())
- CopyDeleted = true;
- else
- MoveDeleted = true;
- }
- }
-
- // If all trivial copy and move constructors are deleted, we cannot copy the
- // argument.
- return !(CopyDeleted && MoveDeleted);
+ return RD->hasNonDeletedCopyOrMoveConstructor();
}
llvm::Constant *CGCXXABI::GetBogusMemberPointer(QualType T) {
Index: lib/AST/DeclCXX.cpp
===================================================================
--- lib/AST/DeclCXX.cpp
+++ lib/AST/DeclCXX.cpp
@@ -64,6 +64,7 @@
DeclaredNonTrivialSpecialMembers(0), HasIrrelevantDestructor(true),
HasConstexprNonCopyMoveConstructor(false),
HasDefaultedDefaultConstructor(false),
+ HasNonDeletedCopyOrMoveConstructor(false),
DefaultedDefaultConstructorIsConstexpr(true),
HasConstexprDefaultConstructor(false),
HasNonLiteralTypeFieldsOrBases(false), ComputedVisibleConversions(false),
@@ -1445,12 +1446,15 @@
}
void CXXRecordDecl::completeDefinition() {
- completeDefinition(nullptr);
+ completeDefinition(nullptr, /*HasNonDeletedCopyOrMoveCtor*/true);
}
-void CXXRecordDecl::completeDefinition(CXXFinalOverriderMap *FinalOverriders) {
+void CXXRecordDecl::completeDefinition(CXXFinalOverriderMap *FinalOverriders,
+ bool HasNonDeletedCopyOrMoveCtor) {
RecordDecl::completeDefinition();
-
+
+ data().HasNonDeletedCopyOrMoveConstructor = HasNonDeletedCopyOrMoveCtor;
+
// If the class may be abstract (but hasn't been marked as such), check for
// any pure final overriders.
if (mayBeAbstract()) {
Index: lib/AST/ASTImporter.cpp
===================================================================
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -973,6 +973,8 @@
= FromData.HasConstexprNonCopyMoveConstructor;
ToData.HasDefaultedDefaultConstructor
= FromData.HasDefaultedDefaultConstructor;
+ ToData.HasNonDeletedCopyOrMoveConstructor
+ = FromData.HasNonDeletedCopyOrMoveConstructor;
ToData.DefaultedDefaultConstructorIsConstexpr
= FromData.DefaultedDefaultConstructorIsConstexpr;
ToData.HasConstexprDefaultConstructor
Index: include/clang/AST/DeclCXX.h
===================================================================
--- include/clang/AST/DeclCXX.h
+++ include/clang/AST/DeclCXX.h
@@ -415,6 +415,10 @@
/// constructor.
unsigned HasDefaultedDefaultConstructor : 1;
+ /// \brief True if this class has at least one non-deleted copy or move
+ /// constructor.
+ unsigned HasNonDeletedCopyOrMoveConstructor : 1;
+
/// \brief True if a defaulted default constructor for this class would
/// be constexpr.
unsigned DefaultedDefaultConstructorIsConstexpr : 1;
@@ -1316,6 +1320,12 @@
return data().HasIrrelevantDestructor;
}
+ /// \brief Determine whether this class has at least one trivial, non-deleted
+ /// copy or move constructor.
+ bool hasNonDeletedCopyOrMoveConstructor() const {
+ return data().HasNonDeletedCopyOrMoveConstructor;
+ }
+
/// \brief Determine whether this class has a non-literal or/ volatile type
/// non-static data member or base class.
bool hasNonLiteralTypeFieldsOrBases() const {
@@ -1682,7 +1692,10 @@
/// be provided as an optimization for abstract-class checking. If NULL,
/// final overriders will be computed if they are needed to complete the
/// definition.
- void completeDefinition(CXXFinalOverriderMap *FinalOverriders);
+ /// \HasNonDeletedCopyOrMoveCtor flips the bit telling the class has a non
+ /// deleted copy or move constructor.
+ void completeDefinition(CXXFinalOverriderMap *FinalOverriders,
+ bool HasNonDeletedCopyOrMoveCtor);
/// \brief Determine whether this class may end up being abstract, even though
/// it is not yet known to be abstract.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
