From d3861ce75a308c65b58c0159e2cee58aea2dff1c Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Sun, 10 Jun 2012 07:07:24 +0000 Subject: [PATCH] Remove CXXRecordDecl flags which are unused after r158289. We need an efficient mechanism to determine whether a defaulted default constructor is constexpr, in order to determine whether a class is a literal type, so keep the incrementally-built form on CXXRecordDecl. Remove the on-demand computation of same, so that we only have one method for determining whether a default constructor is constexpr. This doesn't affect correctness, since default constructor lookup is much simpler than selecting a constructor for copying or moving. We don't need a corresponding mechanism for defaulted copy or move constructors, since they can't affect whether a type is a literal type. Conversely, checking whether such functions are constexpr can require non-trivial effort, so we defer such checks until the copy or move constructor is required. Thus we now only compute whether a copy or move constructor is constexpr on demand, and only compute whether a default constructor is constexpr in advance. This is unfortunate, but seems like the best solution. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@158290 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/DeclCXX.h | 44 ----------------- lib/AST/ASTImporter.cpp | 6 --- lib/AST/DeclCXX.cpp | 76 +++-------------------------- lib/Sema/SemaDeclCXX.cpp | 40 ++++++--------- lib/Serialization/ASTReaderDecl.cpp | 4 -- lib/Serialization/ASTWriter.cpp | 4 -- 6 files changed, 22 insertions(+), 152 deletions(-) diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h index 21e8a98bef..70cd5d52fb 100644 --- a/include/clang/AST/DeclCXX.h +++ b/include/clang/AST/DeclCXX.h @@ -385,26 +385,10 @@ class CXXRecordDecl : public RecordDecl { /// constructor for this class would be constexpr. bool DefaultedDefaultConstructorIsConstexpr : 1; - /// DefaultedCopyConstructorIsConstexpr - True if a defaulted copy - /// constructor for this class would be constexpr. - bool DefaultedCopyConstructorIsConstexpr : 1; - - /// DefaultedMoveConstructorIsConstexpr - True if a defaulted move - /// constructor for this class would be constexpr. - bool DefaultedMoveConstructorIsConstexpr : 1; - /// HasConstexprDefaultConstructor - True if this class has a constexpr /// default constructor (either user-declared or implicitly declared). bool HasConstexprDefaultConstructor : 1; - /// HasConstexprCopyConstructor - True if this class has a constexpr copy - /// constructor (either user-declared or implicitly declared). - bool HasConstexprCopyConstructor : 1; - - /// HasConstexprMoveConstructor - True if this class has a constexpr move - /// constructor (either user-declared or implicitly declared). - bool HasConstexprMoveConstructor : 1; - /// HasTrivialCopyConstructor - True when this class has a trivial copy /// constructor. /// @@ -1105,18 +1089,6 @@ public: (!isUnion() || hasInClassInitializer()); } - /// defaultedCopyConstructorIsConstexpr - Whether a defaulted copy - /// constructor for this class would be constexpr. - bool defaultedCopyConstructorIsConstexpr() const { - return data().DefaultedCopyConstructorIsConstexpr; - } - - /// defaultedMoveConstructorIsConstexpr - Whether a defaulted move - /// constructor for this class would be constexpr. - bool defaultedMoveConstructorIsConstexpr() const { - return data().DefaultedMoveConstructorIsConstexpr; - } - /// hasConstexprDefaultConstructor - Whether this class has a constexpr /// default constructor. bool hasConstexprDefaultConstructor() const { @@ -1125,22 +1097,6 @@ public: defaultedDefaultConstructorIsConstexpr()); } - /// hasConstexprCopyConstructor - Whether this class has a constexpr copy - /// constructor. - bool hasConstexprCopyConstructor() const { - return data().HasConstexprCopyConstructor || - (!data().DeclaredCopyConstructor && - data().DefaultedCopyConstructorIsConstexpr); - } - - /// hasConstexprMoveConstructor - Whether this class has a constexpr move - /// constructor. - bool hasConstexprMoveConstructor() const { - return data().HasConstexprMoveConstructor || - (needsImplicitMoveConstructor() && - data().DefaultedMoveConstructorIsConstexpr); - } - // hasTrivialCopyConstructor - Whether this class has a trivial copy // constructor (C++ [class.copy]p6, C++0x [class.copy]p13) bool hasTrivialCopyConstructor() const { diff --git a/lib/AST/ASTImporter.cpp b/lib/AST/ASTImporter.cpp index d8550eb0f2..bc139207ce 100644 --- a/lib/AST/ASTImporter.cpp +++ b/lib/AST/ASTImporter.cpp @@ -1858,14 +1858,8 @@ bool ASTNodeImporter::ImportDefinition(RecordDecl *From, RecordDecl *To, = FromData.HasConstexprNonCopyMoveConstructor; ToData.DefaultedDefaultConstructorIsConstexpr = FromData.DefaultedDefaultConstructorIsConstexpr; - ToData.DefaultedCopyConstructorIsConstexpr - = FromData.DefaultedCopyConstructorIsConstexpr; - ToData.DefaultedMoveConstructorIsConstexpr - = FromData.DefaultedMoveConstructorIsConstexpr; ToData.HasConstexprDefaultConstructor = FromData.HasConstexprDefaultConstructor; - ToData.HasConstexprCopyConstructor = FromData.HasConstexprCopyConstructor; - ToData.HasConstexprMoveConstructor = FromData.HasConstexprMoveConstructor; ToData.HasTrivialCopyConstructor = FromData.HasTrivialCopyConstructor; ToData.HasTrivialMoveConstructor = FromData.HasTrivialMoveConstructor; ToData.HasTrivialCopyAssignment = FromData.HasTrivialCopyAssignment; diff --git a/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp index 84aa7c5c88..0e28a5e7dd 100644 --- a/lib/AST/DeclCXX.cpp +++ b/lib/AST/DeclCXX.cpp @@ -47,10 +47,7 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D) HasTrivialDefaultConstructor(true), HasConstexprNonCopyMoveConstructor(false), DefaultedDefaultConstructorIsConstexpr(true), - DefaultedCopyConstructorIsConstexpr(true), - DefaultedMoveConstructorIsConstexpr(true), - HasConstexprDefaultConstructor(false), HasConstexprCopyConstructor(false), - HasConstexprMoveConstructor(false), HasTrivialCopyConstructor(true), + HasConstexprDefaultConstructor(false), HasTrivialCopyConstructor(true), HasTrivialMoveConstructor(true), HasTrivialCopyAssignment(true), HasTrivialMoveAssignment(true), HasTrivialDestructor(true), HasIrrelevantDestructor(true), @@ -220,8 +217,6 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases, // In the definition of a constexpr constructor [...] // -- the class shall not have any virtual base classes data().DefaultedDefaultConstructorIsConstexpr = false; - data().DefaultedCopyConstructorIsConstexpr = false; - data().DefaultedMoveConstructorIsConstexpr = false; } else { // C++ [class.ctor]p5: // A default constructor is trivial [...] if: @@ -260,25 +255,6 @@ CXXRecordDecl::setBases(CXXBaseSpecifier const * const *Bases, // default constructor is constexpr. if (!BaseClassDecl->hasConstexprDefaultConstructor()) data().DefaultedDefaultConstructorIsConstexpr = false; - - // C++11 [class.copy]p13: - // If the implicitly-defined constructor would satisfy the requirements - // of a constexpr constructor, the implicitly-defined constructor is - // constexpr. - // C++11 [dcl.constexpr]p4: - // -- every constructor involved in initializing [...] base class - // sub-objects shall be a constexpr constructor - if (!BaseClassDecl->hasConstexprCopyConstructor()) - data().DefaultedCopyConstructorIsConstexpr = false; - if (BaseClassDecl->hasDeclaredMoveConstructor() || - BaseClassDecl->needsImplicitMoveConstructor()) - // FIXME: If the implicit move constructor generated for the base class - // would be ill-formed, the implicit move constructor generated for the - // derived class calls the base class' copy constructor. - data().DefaultedMoveConstructorIsConstexpr &= - BaseClassDecl->hasConstexprMoveConstructor(); - else if (!BaseClassDecl->hasConstexprCopyConstructor()) - data().DefaultedMoveConstructorIsConstexpr = false; } // C++ [class.ctor]p3: @@ -471,11 +447,7 @@ void CXXRecordDecl::markedVirtualFunctionPure() { } void CXXRecordDecl::markedConstructorConstexpr(CXXConstructorDecl *CD) { - if (CD->isCopyConstructor()) - data().HasConstexprCopyConstructor = true; - else if (CD->isMoveConstructor()) - data().HasConstexprMoveConstructor = true; - else + if (!CD->isCopyOrMoveConstructor()) data().HasConstexprNonCopyMoveConstructor = true; if (CD->isDefaultConstructor()) @@ -558,12 +530,8 @@ void CXXRecordDecl::addedMember(Decl *D) { } } else if (Constructor->isCopyConstructor()) { data().DeclaredCopyConstructor = true; - if (Constructor->isConstexpr()) - data().HasConstexprCopyConstructor = true; } else if (Constructor->isMoveConstructor()) { data().DeclaredMoveConstructor = true; - if (Constructor->isConstexpr()) - data().HasConstexprMoveConstructor = true; } else goto NotASpecialMember; return; @@ -620,9 +588,6 @@ NotASpecialMember:; // user-provided [...] if (UserProvided) data().HasTrivialCopyConstructor = false; - - if (Constructor->isConstexpr()) - data().HasConstexprCopyConstructor = true; } else if (Constructor->isMoveConstructor()) { data().UserDeclaredMoveConstructor = true; data().DeclaredMoveConstructor = true; @@ -632,9 +597,6 @@ NotASpecialMember:; // user-provided [...] if (UserProvided) data().HasTrivialMoveConstructor = false; - - if (Constructor->isConstexpr()) - data().HasConstexprMoveConstructor = true; } } if (Constructor->isConstexpr() && !Constructor->isCopyOrMoveConstructor()) { @@ -676,19 +638,9 @@ NotASpecialMember:; // C++11 [class.dtor]p5: // A destructor is trivial if it is not user-provided and if // -- the destructor is not virtual. - if (DD->isUserProvided() || DD->isVirtual()) { + if (DD->isUserProvided() || DD->isVirtual()) data().HasTrivialDestructor = false; - // C++11 [dcl.constexpr]p1: - // The constexpr specifier shall be applied only to [...] the - // declaration of a static data member of a literal type. - // C++11 [basic.types]p10: - // A type is a literal type if it is [...] a class type that [...] has - // a trivial destructor. - data().DefaultedDefaultConstructorIsConstexpr = false; - data().DefaultedCopyConstructorIsConstexpr = false; - data().DefaultedMoveConstructorIsConstexpr = false; - } - + return; } @@ -939,27 +891,11 @@ NotASpecialMember:; // The standard requires any in-class initializer to be a constant // expression. We consider this to be a defect. data().DefaultedDefaultConstructorIsConstexpr = false; - - if (!FieldRec->hasConstexprCopyConstructor()) - data().DefaultedCopyConstructorIsConstexpr = false; - - if (FieldRec->hasDeclaredMoveConstructor() || - FieldRec->needsImplicitMoveConstructor()) - // FIXME: If the implicit move constructor generated for the member's - // class would be ill-formed, the implicit move constructor generated - // for this class calls the member's copy constructor. - data().DefaultedMoveConstructorIsConstexpr &= - FieldRec->hasConstexprMoveConstructor(); - else if (!FieldRec->hasConstexprCopyConstructor()) - data().DefaultedMoveConstructorIsConstexpr = false; } } else { // Base element type of field is a non-class type. - if (!T->isLiteralType()) { - data().DefaultedDefaultConstructorIsConstexpr = false; - data().DefaultedCopyConstructorIsConstexpr = false; - data().DefaultedMoveConstructorIsConstexpr = false; - } else if (!Field->hasInClassInitializer() && !isUnion()) + if (!T->isLiteralType() || + (!Field->hasInClassInitializer() && !isUnion())) data().DefaultedDefaultConstructorIsConstexpr = false; } diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 648c0824d5..4bd1c3202c 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -3897,8 +3897,17 @@ static bool defaultedSpecialMemberIsConstexpr(Sema &S, CXXRecordDecl *ClassDecl, // In the definition of a constexpr constructor [...] switch (CSM) { case Sema::CXXDefaultConstructor: + // Since default constructor lookup is essentially trivial (and cannot + // involve, for instance, template instantiation), we compute whether a + // defaulted default constructor is constexpr directly within CXXRecordDecl. + // + // This is important for performance; we need to know whether the default + // constructor is constexpr to determine whether the type is a literal type. + return ClassDecl->defaultedDefaultConstructorIsConstexpr(); + case Sema::CXXCopyConstructor: case Sema::CXXMoveConstructor: + // For copy or move constructors, we need to perform overload resolution. break; case Sema::CXXCopyAssignment: @@ -3911,11 +3920,12 @@ static bool defaultedSpecialMemberIsConstexpr(Sema &S, CXXRecordDecl *ClassDecl, // -- if the class is a non-empty union, or for each non-empty anonymous // union member of a non-union class, exactly one non-static data member // shall be initialized; [DR1359] + // + // If we squint, this is guaranteed, since exactly one non-static data member + // will be initialized (if the constructor isn't deleted), we just don't know + // which one. if (ClassDecl->isUnion()) - // FIXME: In the default constructor case, we should check that the - // in-class initializer is actually a constant expression. - return CSM != Sema::CXXDefaultConstructor || - ClassDecl->hasInClassInitializer(); + return true; // -- the class shall not have any virtual base classes; if (ClassDecl->getNumVBases()) @@ -3943,29 +3953,11 @@ static bool defaultedSpecialMemberIsConstexpr(Sema &S, CXXRecordDecl *ClassDecl, F != FEnd; ++F) { if (F->isInvalidDecl()) continue; - if (CSM == Sema::CXXDefaultConstructor && F->hasInClassInitializer()) { - // -- every assignment-expression that is an initializer-clause appearing - // directly or indirectly within a brace-or-equal-initializer for a - // non-static data member [...] shall be a constant expression; - // - // We consider this bullet to be a defect, since it results in this type - // having a non-constexpr default constructor: - // struct S { - // int a = 0; - // int b = a; - // }; - // FIXME: We should still check that the constructor selected for this - // initialization (if any) is constexpr. - } else if (const RecordType *RecordTy = - S.Context.getBaseElementType(F->getType())-> - getAs()) { + if (const RecordType *RecordTy = + S.Context.getBaseElementType(F->getType())->getAs()) { CXXRecordDecl *FieldRecDecl = cast(RecordTy->getDecl()); if (!specialMemberIsConstexpr(S, FieldRecDecl, CSM, ConstArg)) return false; - } else if (CSM == Sema::CXXDefaultConstructor) { - // No in-class initializer, and not a class type. This member isn't going - // to be initialized. - return false; } } diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp index cb14adf439..d9e060c91d 100644 --- a/lib/Serialization/ASTReaderDecl.cpp +++ b/lib/Serialization/ASTReaderDecl.cpp @@ -1094,11 +1094,7 @@ void ASTDeclReader::ReadCXXDefinitionData( Data.HasTrivialDefaultConstructor = Record[Idx++]; Data.HasConstexprNonCopyMoveConstructor = Record[Idx++]; Data.DefaultedDefaultConstructorIsConstexpr = Record[Idx++]; - Data.DefaultedCopyConstructorIsConstexpr = Record[Idx++]; - Data.DefaultedMoveConstructorIsConstexpr = Record[Idx++]; Data.HasConstexprDefaultConstructor = Record[Idx++]; - Data.HasConstexprCopyConstructor = Record[Idx++]; - Data.HasConstexprMoveConstructor = Record[Idx++]; Data.HasTrivialCopyConstructor = Record[Idx++]; Data.HasTrivialMoveConstructor = Record[Idx++]; Data.HasTrivialCopyAssignment = Record[Idx++]; diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index bda665698f..efe1abc5c0 100644 --- a/lib/Serialization/ASTWriter.cpp +++ b/lib/Serialization/ASTWriter.cpp @@ -4313,11 +4313,7 @@ void ASTWriter::AddCXXDefinitionData(const CXXRecordDecl *D, RecordDataImpl &Rec Record.push_back(Data.HasTrivialDefaultConstructor); Record.push_back(Data.HasConstexprNonCopyMoveConstructor); Record.push_back(Data.DefaultedDefaultConstructorIsConstexpr); - Record.push_back(Data.DefaultedCopyConstructorIsConstexpr); - Record.push_back(Data.DefaultedMoveConstructorIsConstexpr); Record.push_back(Data.HasConstexprDefaultConstructor); - Record.push_back(Data.HasConstexprCopyConstructor); - Record.push_back(Data.HasConstexprMoveConstructor); Record.push_back(Data.HasTrivialCopyConstructor); Record.push_back(Data.HasTrivialMoveConstructor); Record.push_back(Data.HasTrivialCopyAssignment);