From 37b8c9ee7cf2b4d5ce3ccd3be1fcadd18a783a57 Mon Sep 17 00:00:00 2001 From: Sean Hunt Date: Mon, 9 May 2011 21:45:35 +0000 Subject: [PATCH] Clean up trivial default constructors now. hasTrivialDefaultConstructor() really really means it now. Also implement a fun standards bug regarding aggregates. Doug, if you'd like, I can un-implement that bug if you think it is truly a defect. The bug is that non-special-member constructors are never considered user-provided, so the following is an aggregate: struct foo { foo(int); }; It's kind of bad, but the solution isn't obvious - should struct foo { foo (int) = delete; }; be an aggregate or not? Lastly, add a missing initialization to FunctionDecl. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@131101 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/Decl.h | 1 + include/clang/AST/DeclCXX.h | 15 ++++-- lib/AST/DeclCXX.cpp | 57 ++++++++++++++++------- lib/Sema/SemaDeclCXX.cpp | 6 +-- lib/Sema/SemaLookup.cpp | 6 +-- lib/Serialization/ASTReaderDecl.cpp | 1 + lib/Serialization/ASTWriter.cpp | 1 + test/SemaCXX/aggregate-initialization.cpp | 4 +- 8 files changed, 61 insertions(+), 30 deletions(-) diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h index 0a081e30fb..193c3286f8 100644 --- a/include/clang/AST/Decl.h +++ b/include/clang/AST/Decl.h @@ -1450,6 +1450,7 @@ protected: IsInline(isInlineSpecified), IsInlineSpecified(isInlineSpecified), IsVirtualAsWritten(false), IsPure(false), HasInheritedPrototype(false), HasWrittenPrototype(true), IsDeleted(false), IsTrivial(false), + IsDefaulted(false), IsExplicitlyDefaulted(false), HasImplicitReturnZero(false), IsLateTemplateParsed(false), EndRangeLoc(NameInfo.getEndLoc()), TemplateOrSpecialization(), diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h index 68bc165e77..4cbbff96ba 100644 --- a/include/clang/AST/DeclCXX.h +++ b/include/clang/AST/DeclCXX.h @@ -337,8 +337,8 @@ class CXXRecordDecl : public RecordDecl { /// HasPublicFields - True when there are private non-static data members. bool HasPublicFields : 1; - /// HasTrivialDefaultConstructor - True when this class has a trivial - /// default constructor. + /// HasTrivialDefaultConstructor - True when, if this class has a default + /// constructor, this default constructor is trivial. /// /// C++0x [class.ctor]p5 /// A default constructor is trivial if it is not user-provided and if @@ -438,6 +438,9 @@ class CXXRecordDecl : public RecordDecl { /// \brief Whether we have already declared the default constructor or /// do not need to have one declared. + bool NeedsImplicitDefaultConstructor : 1; + + /// \brief Whether we have already declared the default constructor. bool DeclaredDefaultConstructor : 1; /// \brief Whether we have already declared the copy constructor. @@ -676,8 +679,8 @@ public: /// declared implicitly or does not need one declared implicitly. /// /// This value is used for lazy creation of default constructors. - bool hasDeclaredDefaultConstructor() const { - return data().DeclaredDefaultConstructor; + bool needsImplicitDefaultConstructor() const { + return data().NeedsImplicitDefaultConstructor; } /// hasConstCopyConstructor - Determines whether this class has a @@ -810,7 +813,9 @@ public: // constructor // (C++0x [class.ctor]p5) bool hasTrivialDefaultConstructor() const { - return data().HasTrivialDefaultConstructor; + return data().HasTrivialDefaultConstructor && + (!data().UserDeclaredConstructor || + data().DeclaredDefaultConstructor); } // hasConstExprNonCopyMoveConstructor - Whether this class has at least one diff --git a/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp index ab097e4b30..a18aeffe57 100644 --- a/lib/AST/DeclCXX.cpp +++ b/lib/AST/DeclCXX.cpp @@ -38,10 +38,10 @@ CXXRecordDecl::DefinitionData::DefinitionData(CXXRecordDecl *D) HasTrivialMoveConstructor(true), HasTrivialCopyAssignment(true), HasTrivialMoveAssignment(true), HasTrivialDestructor(true), HasNonLiteralTypeFieldsOrBases(false), ComputedVisibleConversions(false), - DeclaredDefaultConstructor(false), DeclaredCopyConstructor(false), - DeclaredCopyAssignment(false), DeclaredDestructor(false), - NumBases(0), NumVBases(0), Bases(), VBases(), - Definition(D), FirstFriend(0) { + NeedsImplicitDefaultConstructor(false), DeclaredDefaultConstructor(false), + DeclaredCopyConstructor(false), DeclaredCopyAssignment(false), + DeclaredDestructor(false), NumBases(0), NumVBases(0), Bases(), VBases(), + Definition(D), FirstFriend(0) { } CXXRecordDecl::CXXRecordDecl(Kind K, TagKind TK, DeclContext *DC, @@ -458,8 +458,10 @@ void CXXRecordDecl::addedMember(Decl *D) { if (CXXConstructorDecl *Constructor = dyn_cast(D)) { // If this is the implicit default constructor, note that we have now // declared it. - if (Constructor->isDefaultConstructor()) + if (Constructor->isDefaultConstructor()) { data().DeclaredDefaultConstructor = true; + data().NeedsImplicitDefaultConstructor = true; + } // If this is the implicit copy constructor, note that we have now // declared it. else if (Constructor->isCopyConstructor()) @@ -490,21 +492,21 @@ void CXXRecordDecl::addedMember(Decl *D) { data().UserDeclaredConstructor = true; // Note that we have no need of an implicitly-declared default constructor. - data().DeclaredDefaultConstructor = true; + data().NeedsImplicitDefaultConstructor = true; - // C++ [dcl.init.aggr]p1: - // An aggregate is an array or a class (clause 9) with no - // user-declared constructors (12.1) [...]. - data().Aggregate = false; - - // C++ [class]p4: - // A POD-struct is an aggregate class [...] - data().PlainOldData = false; + // FIXME: Under C++0x, /only/ special member functions may be user-provided. + // This is probably a defect. + bool UserProvided = false; // C++0x [class.ctor]p5: // A default constructor is trivial if it is not user-provided [...] - if (Constructor->isUserProvided()) - data().HasTrivialDefaultConstructor = false; + if (Constructor->isDefaultConstructor()) { + data().DeclaredDefaultConstructor = true; + if (Constructor->isUserProvided()) { + data().HasTrivialDefaultConstructor = false; + UserProvided = true; + } + } // Note when we have a user-declared copy or move constructor, which will // suppress the implicit declaration of those constructors. @@ -516,14 +518,18 @@ void CXXRecordDecl::addedMember(Decl *D) { // C++0x [class.copy]p13: // A copy/move constructor for class X is trivial if it is not // user-provided [...] - if (Constructor->isUserProvided()) + if (Constructor->isUserProvided()) { data().HasTrivialCopyConstructor = false; + UserProvided = true; + } } else if (Constructor->isMoveConstructor()) { // C++0x [class.copy]p13: // A copy/move constructor for class X is trivial if it is not // user-provided [...] - if (Constructor->isUserProvided()) + if (Constructor->isUserProvided()) { data().HasTrivialMoveConstructor = false; + UserProvided = true; + } } } if (Constructor->isConstExpr() && @@ -533,6 +539,21 @@ void CXXRecordDecl::addedMember(Decl *D) { data().HasConstExprNonCopyMoveConstructor = true; } + // C++ [dcl.init.aggr]p1: + // An aggregate is an array or a class with no user-declared + // constructors [...]. + // C++0x [dcl.init.aggr]p1: + // An aggregate is an array or a class with no user-provided + // constructors [...]. + if (!getASTContext().getLangOptions().CPlusPlus0x || UserProvided) + data().Aggregate = false; + + // C++ [class]p4: + // A POD-struct is an aggregate class [...] + // Since the POD bit is meant to be C++03 POD-ness, clear it even if the + // type is technically an aggregate in C++0x since it wouldn't be in 03. + data().PlainOldData = false; + return; } diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index e96fb3760c..837a1bdae2 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -5005,7 +5005,7 @@ CXXConstructorDecl *Sema::DeclareImplicitDefaultConstructor( if (const RecordType *BaseType = B->getType()->getAs()) { CXXRecordDecl *BaseClassDecl = cast(BaseType->getDecl()); - if (!BaseClassDecl->hasDeclaredDefaultConstructor()) + if (!BaseClassDecl->needsImplicitDefaultConstructor()) ExceptSpec.CalledDecl(DeclareImplicitDefaultConstructor(BaseClassDecl)); else if (CXXConstructorDecl *Constructor = getDefaultConstructorUnsafe(*this, BaseClassDecl)) @@ -5019,7 +5019,7 @@ CXXConstructorDecl *Sema::DeclareImplicitDefaultConstructor( B != BEnd; ++B) { if (const RecordType *BaseType = B->getType()->getAs()) { CXXRecordDecl *BaseClassDecl = cast(BaseType->getDecl()); - if (!BaseClassDecl->hasDeclaredDefaultConstructor()) + if (!BaseClassDecl->needsImplicitDefaultConstructor()) ExceptSpec.CalledDecl(DeclareImplicitDefaultConstructor(BaseClassDecl)); else if (CXXConstructorDecl *Constructor = getDefaultConstructorUnsafe(*this, BaseClassDecl)) @@ -5034,7 +5034,7 @@ CXXConstructorDecl *Sema::DeclareImplicitDefaultConstructor( if (const RecordType *RecordTy = Context.getBaseElementType(F->getType())->getAs()) { CXXRecordDecl *FieldClassDecl = cast(RecordTy->getDecl()); - if (!FieldClassDecl->hasDeclaredDefaultConstructor()) + if (!FieldClassDecl->needsImplicitDefaultConstructor()) ExceptSpec.CalledDecl( DeclareImplicitDefaultConstructor(FieldClassDecl)); else if (CXXConstructorDecl *Constructor diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp index 775b1d146e..db8d29c1bb 100644 --- a/lib/Sema/SemaLookup.cpp +++ b/lib/Sema/SemaLookup.cpp @@ -534,7 +534,7 @@ void Sema::ForceDeclarationOfImplicitMembers(CXXRecordDecl *Class) { return; // If the default constructor has not yet been declared, do so now. - if (!Class->hasDeclaredDefaultConstructor()) + if (!Class->needsImplicitDefaultConstructor()) DeclareImplicitDefaultConstructor(Class); // If the copy constructor has not yet been declared, do so now. @@ -581,7 +581,7 @@ static void DeclareImplicitMemberFunctionsWithName(Sema &S, if (const CXXRecordDecl *Record = dyn_cast(DC)) if (Record->getDefinition() && CanDeclareSpecialMemberFunction(S.Context, Record)) { - if (!Record->hasDeclaredDefaultConstructor()) + if (!Record->needsImplicitDefaultConstructor()) S.DeclareImplicitDefaultConstructor( const_cast(Record)); if (!Record->hasDeclaredCopyConstructor()) @@ -2140,7 +2140,7 @@ void Sema::LookupOverloadedOperatorName(OverloadedOperatorKind Op, Scope *S, DeclContext::lookup_result Sema::LookupConstructors(CXXRecordDecl *Class) { // If the copy constructor has not yet been declared, do so now. if (CanDeclareSpecialMemberFunction(Context, Class)) { - if (!Class->hasDeclaredDefaultConstructor()) + if (!Class->needsImplicitDefaultConstructor()) DeclareImplicitDefaultConstructor(Class); if (!Class->hasDeclaredCopyConstructor()) DeclareImplicitCopyConstructor(Class); diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp index c31e9e57e4..29ab396f2c 100644 --- a/lib/Serialization/ASTReaderDecl.cpp +++ b/lib/Serialization/ASTReaderDecl.cpp @@ -867,6 +867,7 @@ void ASTDeclReader::ReadCXXDefinitionData( Data.HasTrivialDestructor = Record[Idx++]; Data.HasNonLiteralTypeFieldsOrBases = Record[Idx++]; Data.ComputedVisibleConversions = Record[Idx++]; + Data.NeedsImplicitDefaultConstructor = Record[Idx++]; Data.DeclaredDefaultConstructor = Record[Idx++]; Data.DeclaredCopyConstructor = Record[Idx++]; Data.DeclaredCopyAssignment = Record[Idx++]; diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index 13b333d5fa..21c452edf6 100644 --- a/lib/Serialization/ASTWriter.cpp +++ b/lib/Serialization/ASTWriter.cpp @@ -3827,6 +3827,7 @@ void ASTWriter::AddCXXDefinitionData(const CXXRecordDecl *D, RecordDataImpl &Rec Record.push_back(Data.HasTrivialDestructor); Record.push_back(Data.HasNonLiteralTypeFieldsOrBases); Record.push_back(Data.ComputedVisibleConversions); + Record.push_back(Data.NeedsImplicitDefaultConstructor); Record.push_back(Data.DeclaredDefaultConstructor); Record.push_back(Data.DeclaredCopyConstructor); Record.push_back(Data.DeclaredCopyAssignment); diff --git a/test/SemaCXX/aggregate-initialization.cpp b/test/SemaCXX/aggregate-initialization.cpp index 4c34447940..b9e69b00b7 100644 --- a/test/SemaCXX/aggregate-initialization.cpp +++ b/test/SemaCXX/aggregate-initialization.cpp @@ -2,6 +2,8 @@ // Verify that we can't initialize non-aggregates with an initializer // list. +// FIXME: Note that due to a (likely) standard bug, this is technically an +// aggregate. struct NonAggr1 { NonAggr1(int) { } @@ -22,7 +24,7 @@ struct NonAggr4 { virtual void f(); }; -NonAggr1 na1 = { 17 }; // expected-error{{non-aggregate type 'NonAggr1' cannot be initialized with an initializer list}} +NonAggr1 na1 = { 17 }; NonAggr2 na2 = { 17 }; // expected-error{{non-aggregate type 'NonAggr2' cannot be initialized with an initializer list}} NonAggr3 na3 = { 17 }; // expected-error{{non-aggregate type 'NonAggr3' cannot be initialized with an initializer list}} NonAggr4 na4 = { 17 }; // expected-error{{non-aggregate type 'NonAggr4' cannot be initialized with an initializer list}} -- 2.40.0