From: Douglas Gregor Date: Wed, 29 Sep 2010 00:15:42 +0000 (+0000) Subject: Move the maintenance of CXXRecordDecl::DefinitionData's Abstract bit X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7a39dd01edc43aa5f058e7259a39737fc1f43792;p=clang Move the maintenance of CXXRecordDecl::DefinitionData's Abstract bit completely into CXXRecordDecl, by adding a new completeDefinition() function. This required a little reshuffling of the final-overrider checking code, since the "abstract" calculation in the presence of abstract base classes needs to occur in CXXRecordDecl::completeDefinition() but we don't want to compute final overriders more than one in the common case. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@115007 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h index 3e1f4bae2d..c0b8681024 100644 --- a/include/clang/AST/Decl.h +++ b/include/clang/AST/Decl.h @@ -1862,6 +1862,11 @@ protected: typedef Redeclarable redeclarable_base; virtual TagDecl *getNextRedeclaration() { return RedeclLink.getNext(); } + /// @brief Completes the definition of this tag declaration. + /// + /// This is a helper function for derived classes. + void completeDefinition(); + public: typedef redeclarable_base::redecl_iterator redecl_iterator; redecl_iterator redecls_begin() const { @@ -1926,9 +1931,6 @@ public: /// where it is in the process of being defined. void startDefinition(); - /// @brief Completes the definition of this tag declaration. - void completeDefinition(); - /// getDefinition - Returns the TagDecl that actually defines this /// struct/union/class/enum. When determining whether or not a /// struct/union/class/enum is completely defined, one should use this method @@ -2247,7 +2249,7 @@ public: /// completeDefinition - Notes that the definition of this type is /// now complete. - void completeDefinition(); + virtual void completeDefinition(); static bool classof(const Decl *D) { return classofKind(D->getKind()); } static bool classof(const RecordDecl *D) { return true; } diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h index cdc6d8cf92..052cbe8cb4 100644 --- a/include/clang/AST/DeclCXX.h +++ b/include/clang/AST/DeclCXX.h @@ -711,9 +711,6 @@ public: /// which means that the class contains or inherits a pure virtual function. bool isAbstract() const { return data().Abstract; } - /// setAbstract - Set whether this class is abstract (C++ [class.abstract]) - void setAbstract(bool Abs) { data().Abstract = Abs; } - // hasTrivialConstructor - Whether this class has a trivial constructor // (C++ [class.ctor]p5) bool hasTrivialConstructor() const { return data().HasTrivialConstructor; } @@ -982,6 +979,27 @@ public: return (PathAccess > DeclAccess ? PathAccess : DeclAccess); } + /// \brief Indicates that the definition of this class is now complete. + virtual void completeDefinition(); + + /// \brief Indicates that the definition of this class is now complete, + /// and provides a final overrider map to help determine + /// + /// \param FinalOverriders The final overrider map for this class, which can + /// 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); + + /// \brief Determine whether this class may end up being abstract, even though + /// it is not yet known to be abstract. + /// + /// \returns true if this class is not known to be abstract but has any + /// base classes that are abstract. In this case, \c completeDefinition() + /// will need to compute final overriders to determine whether the class is + /// actually abstract. + bool mayBeAbstract() const; + static bool classof(const Decl *D) { return classofKind(D->getKind()); } static bool classofKind(Kind K) { return K >= firstCXXRecord && K <= lastCXXRecord; diff --git a/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp index 036292b6a5..6e879ee9ee 100644 --- a/lib/AST/DeclCXX.cpp +++ b/lib/AST/DeclCXX.cpp @@ -14,6 +14,7 @@ #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclTemplate.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/CXXInheritance.h" #include "clang/AST/Expr.h" #include "clang/AST/TypeLoc.h" #include "clang/Basic/IdentifierTable.h" @@ -794,6 +795,63 @@ CXXDestructorDecl *CXXRecordDecl::getDestructor() const { return Dtor; } +void CXXRecordDecl::completeDefinition() { + completeDefinition(0); +} + +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()) { + CXXFinalOverriderMap MyFinalOverriders; + if (!FinalOverriders) { + getFinalOverriders(MyFinalOverriders); + FinalOverriders = &MyFinalOverriders; + } + + bool Done = false; + for (CXXFinalOverriderMap::iterator M = FinalOverriders->begin(), + MEnd = FinalOverriders->end(); + M != MEnd && !Done; ++M) { + for (OverridingMethods::iterator SO = M->second.begin(), + SOEnd = M->second.end(); + SO != SOEnd && !Done; ++SO) { + assert(SO->second.size() > 0 && + "All virtual functions have overridding virtual functions"); + + // C++ [class.abstract]p4: + // A class is abstract if it contains or inherits at least one + // pure virtual function for which the final overrider is pure + // virtual. + if (SO->second.front().Method->isPure()) { + data().Abstract = true; + Done = true; + break; + } + } + } + } +} + +bool CXXRecordDecl::mayBeAbstract() const { + if (data().Abstract || isInvalidDecl() || !data().Polymorphic || + isDependentContext()) + return false; + + for (CXXRecordDecl::base_class_const_iterator B = bases_begin(), + BEnd = bases_end(); + B != BEnd; ++B) { + CXXRecordDecl *BaseDecl + = cast(B->getType()->getAs()->getDecl()); + if (BaseDecl->isAbstract()) + return true; + } + + return false; +} + CXXMethodDecl * CXXMethodDecl::Create(ASTContext &C, CXXRecordDecl *RD, const DeclarationNameInfo &NameInfo, diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 871cb9fd78..7902ce84b5 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -6692,7 +6692,64 @@ void Sema::ActOnFields(Scope* S, // Okay, we successfully defined 'Record'. if (Record) { - Record->completeDefinition(); + bool Completed = false; + if (CXXRecordDecl *CXXRecord = dyn_cast(Record)) { + if (!CXXRecord->isInvalidDecl()) { + // Set access bits correctly on the directly-declared conversions. + UnresolvedSetImpl *Convs = CXXRecord->getConversionFunctions(); + for (UnresolvedSetIterator I = Convs->begin(), E = Convs->end(); + I != E; ++I) + Convs->setAccess(I, (*I)->getAccess()); + + if (!CXXRecord->isDependentType()) { + // Add any implicitly-declared members to this class. + AddImplicitlyDeclaredMembersToClass(CXXRecord); + + // If we have virtual base classes, we may end up finding multiple + // final overriders for a given virtual function. Check for this + // problem now. + if (CXXRecord->getNumVBases()) { + CXXFinalOverriderMap FinalOverriders; + CXXRecord->getFinalOverriders(FinalOverriders); + + for (CXXFinalOverriderMap::iterator M = FinalOverriders.begin(), + MEnd = FinalOverriders.end(); + M != MEnd; ++M) { + for (OverridingMethods::iterator SO = M->second.begin(), + SOEnd = M->second.end(); + SO != SOEnd; ++SO) { + assert(SO->second.size() > 0 && + "Virtual function without overridding functions?"); + if (SO->second.size() == 1) + continue; + + // C++ [class.virtual]p2: + // In a derived class, if a virtual member function of a base + // class subobject has more than one final overrider the + // program is ill-formed. + Diag(Record->getLocation(), diag::err_multiple_final_overriders) + << (NamedDecl *)M->first << Record; + Diag(M->first->getLocation(), + diag::note_overridden_virtual_function); + for (OverridingMethods::overriding_iterator + OM = SO->second.begin(), + OMEnd = SO->second.end(); + OM != OMEnd; ++OM) + Diag(OM->Method->getLocation(), diag::note_final_overrider) + << (NamedDecl *)M->first << OM->Method->getParent(); + + Record->setInvalidDecl(); + } + } + CXXRecord->completeDefinition(&FinalOverriders); + Completed = true; + } + } + } + } + + if (!Completed) + Record->completeDefinition(); } else { ObjCIvarDecl **ClsFields = reinterpret_cast(RecFields.data()); diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 425389a6b4..e0049e636f 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -2505,84 +2505,9 @@ static void CheckAbstractClassUsage(AbstractUsageInfo &Info, /// completing, introducing implicitly-declared members, checking for /// abstract types, etc. void Sema::CheckCompletedCXXClass(CXXRecordDecl *Record) { - if (!Record || Record->isInvalidDecl()) + if (!Record) return; - if (!Record->isDependentType()) - AddImplicitlyDeclaredMembersToClass(Record); - - if (Record->isInvalidDecl()) - return; - - // Set access bits correctly on the directly-declared conversions. - UnresolvedSetImpl *Convs = Record->getConversionFunctions(); - for (UnresolvedSetIterator I = Convs->begin(), E = Convs->end(); I != E; ++I) - Convs->setAccess(I, (*I)->getAccess()); - - // Determine whether we need to check for final overriders. We do - // this either when there are virtual base classes (in which case we - // may end up finding multiple final overriders for a given virtual - // function) or any of the base classes is abstract (in which case - // we might detect that this class is abstract). - bool CheckFinalOverriders = false; - if (Record->isPolymorphic() && !Record->isInvalidDecl() && - !Record->isDependentType()) { - if (Record->getNumVBases()) - CheckFinalOverriders = true; - else if (!Record->isAbstract()) { - for (CXXRecordDecl::base_class_const_iterator B = Record->bases_begin(), - BEnd = Record->bases_end(); - B != BEnd; ++B) { - CXXRecordDecl *BaseDecl - = cast(B->getType()->getAs()->getDecl()); - if (BaseDecl->isAbstract()) { - CheckFinalOverriders = true; - break; - } - } - } - } - - if (CheckFinalOverriders) { - CXXFinalOverriderMap FinalOverriders; - Record->getFinalOverriders(FinalOverriders); - - for (CXXFinalOverriderMap::iterator M = FinalOverriders.begin(), - MEnd = FinalOverriders.end(); - M != MEnd; ++M) { - for (OverridingMethods::iterator SO = M->second.begin(), - SOEnd = M->second.end(); - SO != SOEnd; ++SO) { - assert(SO->second.size() > 0 && - "All virtual functions have overridding virtual functions"); - if (SO->second.size() == 1) { - // C++ [class.abstract]p4: - // A class is abstract if it contains or inherits at least one - // pure virtual function for which the final overrider is pure - // virtual. - if (SO->second.front().Method->isPure()) - Record->setAbstract(true); - continue; - } - - // C++ [class.virtual]p2: - // In a derived class, if a virtual member function of a base - // class subobject has more than one final overrider the - // program is ill-formed. - Diag(Record->getLocation(), diag::err_multiple_final_overriders) - << (NamedDecl *)M->first << Record; - Diag(M->first->getLocation(), diag::note_overridden_virtual_function); - for (OverridingMethods::overriding_iterator OM = SO->second.begin(), - OMEnd = SO->second.end(); - OM != OMEnd; ++OM) - Diag(OM->Method->getLocation(), diag::note_final_overrider) - << (NamedDecl *)M->first << OM->Method->getParent(); - - Record->setInvalidDecl(); - } - } - } - if (Record->isAbstract() && !Record->isInvalidDecl()) { AbstractUsageInfo Info(*this, Record); CheckAbstractClassUsage(Info, Record);