[cfe-commits] r115007 - in /cfe/trunk: include/clang/AST/Decl.h include/clang/AST/DeclCXX.h lib/AST/DeclCXX.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp

Douglas Gregor dgregor at apple.com
Tue Sep 28 17:15:42 PDT 2010


Author: dgregor
Date: Tue Sep 28 19:15:42 2010
New Revision: 115007

URL: http://llvm.org/viewvc/llvm-project?rev=115007&view=rev
Log:
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.

Modified:
    cfe/trunk/include/clang/AST/Decl.h
    cfe/trunk/include/clang/AST/DeclCXX.h
    cfe/trunk/lib/AST/DeclCXX.cpp
    cfe/trunk/lib/Sema/SemaDecl.cpp
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=115007&r1=115006&r2=115007&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Tue Sep 28 19:15:42 2010
@@ -1862,6 +1862,11 @@
   typedef Redeclarable<TagDecl> 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 @@
   /// 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 @@
 
   /// 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; }

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=115007&r1=115006&r2=115007&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Sep 28 19:15:42 2010
@@ -711,9 +711,6 @@
   /// 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 @@
     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;

Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=115007&r1=115006&r2=115007&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Tue Sep 28 19:15:42 2010
@@ -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 @@
   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<CXXRecordDecl>(B->getType()->getAs<RecordType>()->getDecl());
+    if (BaseDecl->isAbstract())
+      return true;
+  }
+  
+  return false;
+}
+
 CXXMethodDecl *
 CXXMethodDecl::Create(ASTContext &C, CXXRecordDecl *RD,
                       const DeclarationNameInfo &NameInfo,

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=115007&r1=115006&r2=115007&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Sep 28 19:15:42 2010
@@ -6692,7 +6692,64 @@
 
   // Okay, we successfully defined 'Record'.
   if (Record) {
-    Record->completeDefinition();
+    bool Completed = false;
+    if (CXXRecordDecl *CXXRecord = dyn_cast<CXXRecordDecl>(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<ObjCIvarDecl**>(RecFields.data());

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=115007&r1=115006&r2=115007&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Sep 28 19:15:42 2010
@@ -2505,84 +2505,9 @@
 /// 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<CXXRecordDecl>(B->getType()->getAs<RecordType>()->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);





More information about the cfe-commits mailing list