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

Douglas Gregor dgregor at apple.com
Fri Nov 9 17:18:17 PST 2012


Author: dgregor
Date: Fri Nov  9 19:18:17 2012
New Revision: 167651

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

Modified:
    cfe/trunk/include/clang/AST/DeclCXX.h
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    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=167651&r1=167650&r2=167651&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Fri Nov  9 19:18:17 2012
@@ -1322,8 +1322,12 @@
   /// \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.

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=167651&r1=167650&r2=167651&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Nov  9 19:18:17 2012
@@ -5275,6 +5275,8 @@
 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">;

Modified: cfe/trunk/lib/AST/CXXInheritance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CXXInheritance.cpp?rev=167651&r1=167650&r2=167651&view=diff
==============================================================================
--- cfe/trunk/lib/AST/CXXInheritance.cpp (original)
+++ cfe/trunk/lib/AST/CXXInheritance.cpp Fri Nov  9 19:18:17 2012
@@ -123,7 +123,8 @@
 
 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 @@
   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;

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=167651&r1=167650&r2=167651&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Fri Nov  9 19:18:17 2012
@@ -1018,6 +1018,14 @@
     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 @@
       << 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()) {

Modified: cfe/trunk/test/SemaTemplate/dependent-names.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/dependent-names.cpp?rev=167651&r1=167650&r2=167651&view=diff
==============================================================================
--- cfe/trunk/test/SemaTemplate/dependent-names.cpp (original)
+++ cfe/trunk/test/SemaTemplate/dependent-names.cpp Fri Nov  9 19:18:17 2012
@@ -319,8 +319,27 @@
 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() { }
+  };
 }





More information about the cfe-commits mailing list