<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Nov 9, 2012, at 6:53 PM, Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=us-ascii"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>This isn't actually a circular dependence, either. IntervalMap<>::const_iterator is inheriting from std::iterator.</div><div><br></div><div>I don't see anything obviously wrong with your patch, though...</div></div></blockquote><div><br></div>I see the problem in my patch. CXXRecordDecl::forallBases has a ridiculously non-intuitive API.</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>- Doug</div><div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Nov 9, 2012, at 18:21 , Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=us-ascii"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>LLVM itself seems to have circular inheritance. Is this supposed to happen?</div><div><br></div><div><pre style="font-family: 'Courier New', courier, monotype; "><span class="stderr" style="color: red; ">In file included from /Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin10-DA/llvm/lib/Support/IntervalMap.cpp:14:
/Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin10-DA/llvm/include/llvm/ADT/IntervalMap.h:1499:61: error: circular inheritance between 'llvm::IntervalMap::const_iterator' and 'llvm::IntervalMap::iterator'
class IntervalMap<KeyT, ValT, N, Traits>::iterator : public const_iterator {
^
/Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin10-DA/llvm/include/llvm/ADT/IntervalMap.h:1275:43: note: 'llvm::IntervalMap::const_iterator' declared here
class IntervalMap<KeyT, ValT, N, Traits>::const_iterator :
^
</span></pre></div><div>(This is from smooshlab.)</div><div><br></div><br><div><div>On Nov 9, 2012, at 17:18 , Douglas Gregor <<a href="mailto:dgregor@apple.com">dgregor@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Author: dgregor<br>Date: Fri Nov 9 19:18:17 2012<br>New Revision: 167651<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=167651&view=rev">http://llvm.org/viewvc/llvm-project?rev=167651&view=rev</a><br>Log:<br>Diagnostic circular inheritance involving dependent base classes. We<br>would have diagnosed this at instantiation time anyway, if only we<br>didn't hang on all of these test cases. Fixes <<a href="rdar://problem/12629723">rdar://problem/12629723</a>><br><br>Modified:<br> cfe/trunk/include/clang/AST/DeclCXX.h<br> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br> cfe/trunk/lib/AST/CXXInheritance.cpp<br> cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br> cfe/trunk/test/SemaTemplate/dependent-names.cpp<br><br>Modified: cfe/trunk/include/clang/AST/DeclCXX.h<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=167651&r1=167650&r2=167651&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=167651&r1=167650&r2=167651&view=diff</a><br>==============================================================================<br>--- cfe/trunk/include/clang/AST/DeclCXX.h (original)<br>+++ cfe/trunk/include/clang/AST/DeclCXX.h Fri Nov 9 19:18:17 2012<br>@@ -1322,8 +1322,12 @@<br> /// \param AllowShortCircuit if false, forces the callback to be called<br> /// for every base class, even if a dependent or non-matching base was<br> /// found.<br>+ ///<br>+ /// \param VisitDependent whether we should also visit dependent bases<br>+ /// that can be resolved to CXXRecordDecls.<br> bool forallBases(ForallBasesCallback *BaseMatches, void *UserData,<br>- bool AllowShortCircuit = true) const;<br>+ bool AllowShortCircuit = true,<br>+ bool VisitDependent = false) const;<br><br> /// \brief Function type used by lookupInBases() to determine whether a<br> /// specific base class subobject matches the lookup criteria.<br><br>Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=167651&r1=167650&r2=167651&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=167651&r1=167650&r2=167651&view=diff</a><br>==============================================================================<br>--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Nov 9 19:18:17 2012<br>@@ -5275,6 +5275,8 @@<br> def err_base_clause_on_union : Error<"unions cannot have base classes">;<br> def err_base_must_be_class : Error<"base specifier must name a class">;<br> def err_union_as_base_class : Error<"unions cannot be base classes">;<br>+def err_circular_inheritance : Error<<br>+ "circular inheritance between %0 and %1">;<br> def err_incomplete_base_class : Error<"base class has incomplete type">;<br> def err_duplicate_base_class : Error<<br> "base class %0 specified more than once as a direct base class">;<br><br>Modified: cfe/trunk/lib/AST/CXXInheritance.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CXXInheritance.cpp?rev=167651&r1=167650&r2=167651&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CXXInheritance.cpp?rev=167651&r1=167650&r2=167651&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/AST/CXXInheritance.cpp (original)<br>+++ cfe/trunk/lib/AST/CXXInheritance.cpp Fri Nov 9 19:18:17 2012<br>@@ -123,7 +123,8 @@<br><br> bool CXXRecordDecl::forallBases(ForallBasesCallback *BaseMatches,<br> void *OpaqueData,<br>- bool AllowShortCircuit) const {<br>+ bool AllowShortCircuit,<br>+ bool VisitDependent) const {<br> SmallVector<const CXXRecordDecl*, 8> Queue;<br><br> const CXXRecordDecl *Record = this;<br>@@ -131,15 +132,14 @@<br> while (true) {<br> for (CXXRecordDecl::base_class_const_iterator<br> I = Record->bases_begin(), E = Record->bases_end(); I != E; ++I) {<br>- const RecordType *Ty = I->getType()->getAs<RecordType>();<br>- if (!Ty) {<br>+ CXXRecordDecl *Base = I->getType()->getAsCXXRecordDecl();<br>+ if (!Base || (!VisitDependent && I->getType()->isDependentType())) {<br> if (AllowShortCircuit) return false;<br> AllMatches = false;<br> continue;<br> }<br><br>- CXXRecordDecl *Base = <br>- cast_or_null<CXXRecordDecl>(Ty->getDecl()->getDefinition());<br>+ Base = Base->getDefinition();<br> if (!Base) {<br> if (AllowShortCircuit) return false;<br> AllMatches = false;<br><br>Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=167651&r1=167650&r2=167651&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=167651&r1=167650&r2=167651&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)<br>+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Fri Nov 9 19:18:17 2012<br>@@ -1018,6 +1018,14 @@<br> return false;<br> }<br><br>+/// \brief Determine whether we have the same C++ record definition.<br>+///<br>+/// Used as a helper function in Sema::CheckBaseSpecifier, below.<br>+static bool sameCXXRecordDef(const CXXRecordDecl *BaseDefinition,<br>+ void *UserData) {<br>+ return (CXXRecordDecl *)UserData != BaseDefinition;<br>+}<br>+<br> /// \brief Check the validity of a C++ base class specifier.<br> ///<br> /// \returns a new CXXBaseSpecifier if well-formed, emits diagnostics<br>@@ -1044,13 +1052,32 @@<br> << TInfo->getTypeLoc().getSourceRange();<br> EllipsisLoc = SourceLocation();<br> }<br>- <br>- if (BaseType->isDependentType())<br>+<br>+ SourceLocation BaseLoc = TInfo->getTypeLoc().getBeginLoc();<br>+<br>+ if (BaseType->isDependentType()) {<br>+ // Make sure that we don't have circular inheritance among our dependent<br>+ // bases. For non-dependent bases, the check for completeness below handles<br>+ // this.<br>+ if (CXXRecordDecl *BaseDecl = BaseType->getAsCXXRecordDecl()) {<br>+ if (BaseDecl->getCanonicalDecl() == Class->getCanonicalDecl() ||<br>+ ((BaseDecl = BaseDecl->getDefinition()) &&<br>+ !BaseDecl->forallBases(&sameCXXRecordDef, Class))) {<br>+ Diag(BaseLoc, diag::err_circular_inheritance)<br>+ << BaseType << Context.getTypeDeclType(Class);<br>+<br>+ if (BaseDecl->getCanonicalDecl() != Class->getCanonicalDecl())<br>+ Diag(BaseDecl->getLocation(), diag::note_previous_decl)<br>+ << BaseType;<br>+ <br>+ return 0;<br>+ }<br>+ }<br>+<br> return new (Context) CXXBaseSpecifier(SpecifierRange, Virtual,<br> Class->getTagKind() == TTK_Class,<br> Access, TInfo, EllipsisLoc);<br>-<br>- SourceLocation BaseLoc = TInfo->getTypeLoc().getBeginLoc();<br>+ }<br><br> // Base specifiers must be record types.<br> if (!BaseType->isRecordType()) {<br><br>Modified: cfe/trunk/test/SemaTemplate/dependent-names.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/dependent-names.cpp?rev=167651&r1=167650&r2=167651&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/dependent-names.cpp?rev=167651&r1=167650&r2=167651&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/SemaTemplate/dependent-names.cpp (original)<br>+++ cfe/trunk/test/SemaTemplate/dependent-names.cpp Fri Nov 9 19:18:17 2012<br>@@ -319,8 +319,27 @@<br> template < unsigned > struct X {<br> static const unsigned dimension = 3;<br> template<unsigned dim=dimension> <br>- struct Y: Y<dim> { }; // expected-error {{incomplete type}} expected-note {{is not complete until the closing}}<br>+ struct Y: Y<dim> { }; // expected-error{{circular inheritance between 'Y<dim>' and 'Y<dim>'}}<br> };<br> typedef X<3> X3;<br>-X3::Y<>::iterator it; // expected-note {{requested here}}<br>+X3::Y<>::iterator it; // expected-error {{no type named 'iterator' in 'PR11421::X<3>::Y<3>'}}<br>+}<br>+<br>+namespace rdar12629723 {<br>+ template<class T><br>+ struct X {<br>+ struct C : public C { }; // expected-error{{circular inheritance between 'rdar12629723::X::C' and 'rdar12629723::X::C'}}<br>+<br>+ struct B;<br>+<br>+ struct A : public B { // expected-note{{'rdar12629723::X::A' declared here}}<br>+ virtual void foo() { }<br>+ };<br>+ struct B;<br>+ };<br>+<br>+ template<class T><br>+ struct X<T>::B : public A { // expected-error{{circular inheritance between 'rdar12629723::X::A' and 'rdar12629723::X::B'}}<br>+ virtual void foo() { }<br>+ };<br> }<br><br><br>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br></blockquote></div><br></div></blockquote></div><br></div></blockquote></div><br></body></html>