[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
Jordan Rose
jordan_rose at apple.com
Fri Nov 9 18:53:33 PST 2012
This isn't actually a circular dependence, either. IntervalMap<>::const_iterator is inheriting from std::iterator.
I don't see anything obviously wrong with your patch, though...
On Nov 9, 2012, at 18:21 , Jordan Rose <jordan_rose at apple.com> wrote:
> LLVM itself seems to have circular inheritance. Is this supposed to happen?
>
> 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 :
> ^
> (This is from smooshlab.)
>
>
> On Nov 9, 2012, at 17:18 , Douglas Gregor <dgregor at apple.com> wrote:
>
>> 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() { }
>> + };
>> }
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121109/5753b529/attachment.html>
More information about the cfe-commits
mailing list