[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 19:23:31 PST 2012
On Nov 9, 2012, at 6:53 PM, Jordan Rose <jordan_rose at apple.com> wrote:
> 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...
I see the problem in my patch. CXXRecordDecl::forallBases has a ridiculously non-intuitive API.
- Doug
>
> 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/ad8ff4ff/attachment.html>
More information about the cfe-commits
mailing list