From 6200470112118250985191b74468adfd99f3a8cf Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Sat, 10 Nov 2012 01:18:17 +0000 Subject: [PATCH] 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 llvm-svn: 167651 --- clang/include/clang/AST/DeclCXX.h | 6 +++- clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 ++ clang/lib/AST/CXXInheritance.cpp | 10 +++---- clang/lib/Sema/SemaDeclCXX.cpp | 35 +++++++++++++++++++++--- clang/test/SemaTemplate/dependent-names.cpp | 23 ++++++++++++++-- 5 files changed, 64 insertions(+), 12 deletions(-) diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index 9cb56e2..d5d66fc 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/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/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index e6a08b0..0d64bf3 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/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/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp index 213b214..6e91f3f 100644 --- a/clang/lib/AST/CXXInheritance.cpp +++ b/clang/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 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(); - 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(Ty->getDecl()->getDefinition()); + Base = Base->getDefinition(); if (!Base) { if (AllowShortCircuit) return false; AllMatches = false; diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index f9eb9eb..1c3d4f2 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/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/clang/test/SemaTemplate/dependent-names.cpp b/clang/test/SemaTemplate/dependent-names.cpp index 924bad9..4e322d9 100644 --- a/clang/test/SemaTemplate/dependent-names.cpp +++ b/clang/test/SemaTemplate/dependent-names.cpp @@ -319,8 +319,27 @@ namespace PR11421 { template < unsigned > struct X { static const unsigned dimension = 3; template - struct Y: Y { }; // expected-error {{incomplete type}} expected-note {{is not complete until the closing}} + struct Y: Y { }; // expected-error{{circular inheritance between 'Y' and 'Y'}} }; 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 + 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 + struct X::B : public A { // expected-error{{circular inheritance between 'rdar12629723::X::A' and 'rdar12629723::X::B'}} + virtual void foo() { } + }; } -- 2.7.4