[cfe-commits] r167668 - in /cfe/trunk: include/clang/AST/DeclCXX.h lib/AST/CXXInheritance.cpp lib/Sema/SemaDeclCXX.cpp test/SemaTemplate/dependent-names.cpp

[Douglas Gregor]

Author: dgregor
Date: Sat Nov 10 01:24:09 2012
New Revision: 167668

URL: http://llvm.org/viewvc/llvm-project?rev=167668&view=rev
Log:
Rework my implementation of circular-reference finding to not use
CXXRecordDecl::forallBases, which does *not* do what I need. Fixes the
failure introduced in r167651.

Modified:
    cfe/trunk/include/clang/AST/DeclCXX.h
    cfe/trunk/lib/AST/CXXInheritance.cpp
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
    cfe/trunk/test/SemaTemplate/dependent-names.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=167668&r1=167667&r2=167668&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Sat Nov 10 01:24:09 2012
@@ -1322,12 +1322,8 @@
   /// \param AllowShortCircuit if false, forces the callback to be called
   /// for every base class, even if a dependent or non-matching base was
   /// found.
-  ///
-  /// \param VisitDependent whether we should also visit dependent bases
-  /// that can be resolved to CXXRecordDecls.
   bool forallBases(ForallBasesCallback *BaseMatches, void *UserData,
-                   bool AllowShortCircuit = true,
-                   bool VisitDependent = false) const;
+                   bool AllowShortCircuit = true) const;
 
   /// \brief Function type used by lookupInBases() to determine whether a
   /// specific base class subobject matches the lookup criteria.

Modified: cfe/trunk/lib/AST/CXXInheritance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CXXInheritance.cpp?rev=167668&r1=167667&r2=167668&view=diff
==============================================================================
--- cfe/trunk/lib/AST/CXXInheritance.cpp (original)
+++ cfe/trunk/lib/AST/CXXInheritance.cpp Sat Nov 10 01:24:09 2012
@@ -123,8 +123,7 @@
 
 bool CXXRecordDecl::forallBases(ForallBasesCallback *BaseMatches,
                                 void *OpaqueData,
-                                bool AllowShortCircuit,
-                                bool VisitDependent) const {
+                                bool AllowShortCircuit) const {
   SmallVector<const CXXRecordDecl*, 8> Queue;
 
   const CXXRecordDecl *Record = this;
@@ -132,14 +131,15 @@
   while (true) {
     for (CXXRecordDecl::base_class_const_iterator
            I = Record->bases_begin(), E = Record->bases_end(); I != E; ++I) {
-      CXXRecordDecl *Base = I->getType()->getAsCXXRecordDecl();
-      if (!Base || (!VisitDependent && I->getType()->isDependentType())) {
+      const RecordType *Ty = I->getType()->getAs<RecordType>();
+      if (!Ty) {
         if (AllowShortCircuit) return false;
         AllMatches = false;
         continue;
       }
 
-      Base = Base->getDefinition();
+      CXXRecordDecl *Base = 
+            cast_or_null<CXXRecordDecl>(Ty->getDecl()->getDefinition());
       if (!Base) {
         if (AllowShortCircuit) return false;
         AllMatches = false;

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=167668&r1=167667&r2=167668&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Sat Nov 10 01:24:09 2012
@@ -1018,12 +1018,39 @@
     return false;
 }
 
-/// \brief Determine whether we have the same C++ record definition.
-///
-/// Used as a helper function in Sema::CheckBaseSpecifier, below.
-static bool sameCXXRecordDef(const CXXRecordDecl *BaseDefinition,
-                          void *UserData) {
-  return (CXXRecordDecl *)UserData != BaseDefinition;
+/// \brief Determine whether the given class is a base class of the given
+/// class, including looking at dependent bases.
+static bool findCircularInheritance(const CXXRecordDecl *Class,
+                                    const CXXRecordDecl *Current) {
+  SmallVector<const CXXRecordDecl*, 8> Queue;
+
+  Class = Class->getCanonicalDecl();
+  while (true) {
+    for (CXXRecordDecl::base_class_const_iterator I = Current->bases_begin(),
+                                                  E = Current->bases_end();
+         I != E; ++I) {
+      CXXRecordDecl *Base = I->getType()->getAsCXXRecordDecl();
+      if (!Base)
+        continue;
+
+      Base = Base->getDefinition();
+      if (!Base)
+        continue;
+
+      if (Base->getCanonicalDecl() == Class)
+        return true;
+
+      Queue.push_back(Base);
+    }
+
+    if (Queue.empty())
+      return false;
+
+    Current = Queue.back();
+    Queue.pop_back();
+  }
+
+  return false;
 }
 
 /// \brief Check the validity of a C++ base class specifier.
@@ -1062,7 +1089,7 @@
     if (CXXRecordDecl *BaseDecl = BaseType->getAsCXXRecordDecl()) {
       if (BaseDecl->getCanonicalDecl() == Class->getCanonicalDecl() ||
           ((BaseDecl = BaseDecl->getDefinition()) &&
-           !BaseDecl->forallBases(&sameCXXRecordDef, Class))) {
+           findCircularInheritance(Class, BaseDecl))) {
         Diag(BaseLoc, diag::err_circular_inheritance)
           << BaseType << Context.getTypeDeclType(Class);
 

Modified: cfe/trunk/test/SemaTemplate/dependent-names.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/dependent-names.cpp?rev=167668&r1=167667&r2=167668&view=diff
==============================================================================
--- cfe/trunk/test/SemaTemplate/dependent-names.cpp (original)
+++ cfe/trunk/test/SemaTemplate/dependent-names.cpp Sat Nov 10 01:24:09 2012
@@ -336,6 +336,9 @@
       virtual void foo() { }
     };
     struct B;
+
+    struct D : T::foo { };
+    struct E : D { };
   };
 
   template<class T>