Diagnostic circular inheritance involving dependent base classes. We

would have diagnosed this at instantiation time anyway, if only we
didn't hang on all of these test cases. Fixes <rdar://problem/12629723>


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@167651 91177308-0d34-0410-b5e6-96231b3b80d8

5 files changed, 64 insertions, 12 deletions

diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h
index 9cb56e2b3c..d5d66fcd96 100644
--- a/include/clang/AST/DeclCXX.h
+++ b/include/clang/AST/DeclCXX.h
@@ -1322,8 +1322,12 @@ public:
   /// \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) const;
+                   bool AllowShortCircuit = true,
+                   bool VisitDependent = false) const;
 
   /// \brief Function type used by lookupInBases() to determine whether a
   /// specific base class subobject matches the lookup criteria.
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index e6a08b0516..0d64bf38ad 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5275,6 +5275,8 @@ def err_static_data_member_not_allowed_in_local_class : Error<
 def err_base_clause_on_union : Error<"unions cannot have base classes">;
 def err_base_must_be_class : Error<"base specifier must name a class">;
 def err_union_as_base_class : Error<"unions cannot be base classes">;
+def err_circular_inheritance : Error<
+  "circular inheritance between %0 and %1">;
 def err_incomplete_base_class : Error<"base class has incomplete type">;
 def err_duplicate_base_class : Error<
   "base class %0 specified more than once as a direct base class">;
diff --git a/lib/AST/CXXInheritance.cpp b/lib/AST/CXXInheritance.cpp
index 213b214a4e..6e91f3f910 100644
--- a/lib/AST/CXXInheritance.cpp
+++ b/lib/AST/CXXInheritance.cpp
@@ -123,7 +123,8 @@ bool CXXRecordDecl::isProvablyNotDerivedFrom(const CXXRecordDecl *Base) const {
 
 bool CXXRecordDecl::forallBases(ForallBasesCallback *BaseMatches,
                                 void *OpaqueData,
-                                bool AllowShortCircuit) const {
+                                bool AllowShortCircuit,
+                                bool VisitDependent) const {
   SmallVector<const CXXRecordDecl*, 8> Queue;
 
   const CXXRecordDecl *Record = this;
@@ -131,15 +132,14 @@ bool CXXRecordDecl::forallBases(ForallBasesCallback *BaseMatches,
   while (true) {
     for (CXXRecordDecl::base_class_const_iterator
            I = Record->bases_begin(), E = Record->bases_end(); I != E; ++I) {
-      const RecordType *Ty = I->getType()->getAs<RecordType>();
-      if (!Ty) {
+      CXXRecordDecl *Base = I->getType()->getAsCXXRecordDecl();
+      if (!Base || (!VisitDependent && I->getType()->isDependentType())) {
         if (AllowShortCircuit) return false;
         AllMatches = false;
         continue;
       }
 
-      CXXRecordDecl *Base = 
-            cast_or_null<CXXRecordDecl>(Ty->getDecl()->getDefinition());
+      Base = Base->getDefinition();
       if (!Base) {
         if (AllowShortCircuit) return false;
         AllMatches = false;
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index f9eb9ebc99..1c3d4f2052 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -1018,6 +1018,14 @@ bool Sema::isCurrentClassName(const IdentifierInfo &II, Scope *,
     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 Check the validity of a C++ base class specifier.
 ///
 /// \returns a new CXXBaseSpecifier if well-formed, emits diagnostics
@@ -1044,13 +1052,32 @@ Sema::CheckBaseSpecifier(CXXRecordDecl *Class,
       << TInfo->getTypeLoc().getSourceRange();
     EllipsisLoc = SourceLocation();
   }
-  
-  if (BaseType->isDependentType())
+
+  SourceLocation BaseLoc = TInfo->getTypeLoc().getBeginLoc();
+
+  if (BaseType->isDependentType()) {
+    // Make sure that we don't have circular inheritance among our dependent
+    // bases. For non-dependent bases, the check for completeness below handles
+    // this.
+    if (CXXRecordDecl *BaseDecl = BaseType->getAsCXXRecordDecl()) {
+      if (BaseDecl->getCanonicalDecl() == Class->getCanonicalDecl() ||
+          ((BaseDecl = BaseDecl->getDefinition()) &&
+           !BaseDecl->forallBases(&sameCXXRecordDef, Class))) {
+        Diag(BaseLoc, diag::err_circular_inheritance)
+          << BaseType << Context.getTypeDeclType(Class);
+
+        if (BaseDecl->getCanonicalDecl() != Class->getCanonicalDecl())
+          Diag(BaseDecl->getLocation(), diag::note_previous_decl)
+            << BaseType;
+            
+        return 0;
+      }
+    }
+
     return new (Context) CXXBaseSpecifier(SpecifierRange, Virtual,
                                           Class->getTagKind() == TTK_Class,
                                           Access, TInfo, EllipsisLoc);
-
-  SourceLocation BaseLoc = TInfo->getTypeLoc().getBeginLoc();
+  }
 
   // Base specifiers must be record types.
   if (!BaseType->isRecordType()) {
diff --git a/test/SemaTemplate/dependent-names.cpp b/test/SemaTemplate/dependent-names.cpp
index 924bad9257..4e322d909d 100644
--- a/test/SemaTemplate/dependent-names.cpp
+++ b/test/SemaTemplate/dependent-names.cpp
@@ -319,8 +319,27 @@ namespace PR11421 {
 template < unsigned > struct X {
   static const unsigned dimension = 3;
   template<unsigned dim=dimension> 
-  struct Y: Y<dim> { }; // expected-error {{incomplete type}} expected-note {{is not complete until the closing}}
+  struct Y: Y<dim> { }; // expected-error{{circular inheritance between 'Y<dim>' and 'Y<dim>'}}
 };
 typedef X<3> X3;
-X3::Y<>::iterator it; // expected-note {{requested here}}
+X3::Y<>::iterator it; // expected-error {{no type named 'iterator' in 'PR11421::X<3>::Y<3>'}}
+}
+
+namespace rdar12629723 {
+  template<class T>
+  struct X {
+    struct C : public C { }; // expected-error{{circular inheritance between 'rdar12629723::X::C' and 'rdar12629723::X::C'}}
+
+    struct B;
+
+    struct A : public B {  // expected-note{{'rdar12629723::X::A' declared here}}
+      virtual void foo() { }
+    };
+    struct B;
+  };
+
+  template<class T>
+  struct X<T>::B : public A {  // expected-error{{circular inheritance between 'rdar12629723::X::A' and 'rdar12629723::X::B'}}
+    virtual void foo() { }
+  };
 }