From: Argyrios Kyrtzidis Date: Sat, 12 Nov 2011 21:07:46 +0000 (+0000) Subject: [PCH] When completing an objc forward reference, do not serialize the chain of its... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ad834d534e9a5db3d3baa09593775f83ceaff1f2;p=clang [PCH] When completing an objc forward reference, do not serialize the chain of its categories because it is going to be rewritten (and the chain will be serialized again), otherwise we may form a cycle in its categories list when deserializing. Also introduce ASTMutationListener::CompletedObjCForwardRef to notify that a forward reference was completed; using Decl's isChangedSinceDeserialization/setChangedSinceDeserialization is bug inducing and kinda gross, we should phase it out. Fixes infinite loop in rdar://10418538. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@144465 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/ASTMutationListener.h b/include/clang/AST/ASTMutationListener.h index 793d3ee2b1..7f5cbd1b01 100644 --- a/include/clang/AST/ASTMutationListener.h +++ b/include/clang/AST/ASTMutationListener.h @@ -24,6 +24,7 @@ namespace clang { class FunctionTemplateDecl; class ObjCCategoryDecl; class ObjCInterfaceDecl; + class ObjCContainerDecl; /// \brief An abstract interface that should be implemented by listeners /// that want to be notified when an AST entity gets modified after its @@ -60,6 +61,9 @@ public: /// \brief A new objc category class was added for an interface. virtual void AddedObjCCategoryToInterface(const ObjCCategoryDecl *CatD, const ObjCInterfaceDecl *IFD) {} + + /// \brief A objc interface or protocol forward reference was completed. + virtual void CompletedObjCForwardRef(const ObjCContainerDecl *D) {} }; } // end namespace clang diff --git a/include/clang/AST/DeclObjC.h b/include/clang/AST/DeclObjC.h index 2a7849188b..4e65bf049d 100644 --- a/include/clang/AST/DeclObjC.h +++ b/include/clang/AST/DeclObjC.h @@ -710,7 +710,8 @@ public: bool isInitiallyForwardDecl() const { return InitiallyForwardDecl; } bool isForwardDecl() const { return ForwardDecl; } - void setForwardDecl(bool val) { ForwardDecl = val; } + + void completedForwardDecl(); ObjCInterfaceDecl *getSuperClass() const { if (ExternallyCompleted) @@ -1000,7 +1001,8 @@ public: bool isInitiallyForwardDecl() const { return InitiallyForwardDecl; } bool isForwardDecl() const { return isForwardProtoDecl; } - void setForwardDecl(bool val) { isForwardProtoDecl = val; } + + void completedForwardDecl(); // Location information, modeled after the Stmt API. SourceLocation getLocStart() const { return getAtStartLoc(); } // '@'protocol diff --git a/include/clang/Serialization/ASTWriter.h b/include/clang/Serialization/ASTWriter.h index f6a8812b20..bff14ec716 100644 --- a/include/clang/Serialization/ASTWriter.h +++ b/include/clang/Serialization/ASTWriter.h @@ -394,7 +394,6 @@ private: void ResolveDeclUpdatesBlocks(); void WriteDeclUpdatesBlocks(); void WriteDeclReplacementsBlock(); - void ResolveChainedObjCCategories(); void WriteChainedObjCCategories(); void WriteDeclContextVisibleUpdate(const DeclContext *DC); void WriteFPPragmaOptions(const FPOptions &Opts); @@ -592,6 +591,10 @@ public: const_cast(D)->setChangedSinceDeserialization(false); } + bool isRewritten(const Decl *D) const { + return DeclsToRewrite.count(D); + } + /// \brief Note that the identifier II occurs at the given offset /// within the identifier table. void SetIdentifierOffset(const IdentifierInfo *II, uint32_t Offset); @@ -665,6 +668,7 @@ public: virtual void StaticDataMemberInstantiated(const VarDecl *D); virtual void AddedObjCCategoryToInterface(const ObjCCategoryDecl *CatD, const ObjCInterfaceDecl *IFD); + virtual void CompletedObjCForwardRef(const ObjCContainerDecl *D); }; /// \brief AST and semantic-analysis consumer that generates a diff --git a/lib/AST/ASTImporter.cpp b/lib/AST/ASTImporter.cpp index f7a55a1474..318f6f2897 100644 --- a/lib/AST/ASTImporter.cpp +++ b/lib/AST/ASTImporter.cpp @@ -3111,9 +3111,10 @@ Decl *ASTNodeImporter::VisitObjCProtocolDecl(ObjCProtocolDecl *D) { Name.getAsIdentifierInfo(), Loc, Importer.Import(D->getAtStartLoc()), D->isInitiallyForwardDecl()); - ToProto->setForwardDecl(D->isForwardDecl()); ToProto->setLexicalDeclContext(LexicalDC); LexicalDC->addDeclInternal(ToProto); + if (D->isInitiallyForwardDecl() && !D->isForwardDecl()) + ToProto->completedForwardDecl(); } Importer.Imported(D, ToProto); @@ -3172,11 +3173,12 @@ Decl *ASTNodeImporter::VisitObjCInterfaceDecl(ObjCInterfaceDecl *D) { ToIface = ObjCInterfaceDecl::Create(Importer.getToContext(), DC, Importer.Import(D->getAtStartLoc()), Name.getAsIdentifierInfo(), Loc, - D->isForwardDecl(), + D->isInitiallyForwardDecl(), D->isImplicitInterfaceDecl()); - ToIface->setForwardDecl(D->isForwardDecl()); ToIface->setLexicalDeclContext(LexicalDC); LexicalDC->addDeclInternal(ToIface); + if (D->isInitiallyForwardDecl() && !D->isForwardDecl()) + ToIface->completedForwardDecl(); } Importer.Imported(D, ToIface); diff --git a/lib/AST/DeclObjC.cpp b/lib/AST/DeclObjC.cpp index 5c4d25fd02..35ee7c6ccd 100644 --- a/lib/AST/DeclObjC.cpp +++ b/lib/AST/DeclObjC.cpp @@ -217,6 +217,13 @@ void ObjCInterfaceDecl::mergeClassExtensionProtocolList( AllReferencedProtocols.set(ProtocolRefs.data(), ProtocolRefs.size(), C); } +void ObjCInterfaceDecl::completedForwardDecl() { + assert(isForwardDecl() && "Only valid to call for forward refs"); + ForwardDecl = false; + if (ASTMutationListener *L = getASTContext().getASTMutationListener()) + L->CompletedObjCForwardRef(this); +} + /// getFirstClassExtension - Find first class extension of the given class. ObjCCategoryDecl* ObjCInterfaceDecl::getFirstClassExtension() const { for (ObjCCategoryDecl *CDecl = getCategoryList(); CDecl; @@ -913,6 +920,13 @@ ObjCMethodDecl *ObjCProtocolDecl::lookupMethod(Selector Sel, return NULL; } +void ObjCProtocolDecl::completedForwardDecl() { + assert(isForwardDecl() && "Only valid to call for forward refs"); + isForwardProtoDecl = false; + if (ASTMutationListener *L = getASTContext().getASTMutationListener()) + L->CompletedObjCForwardRef(this); +} + //===----------------------------------------------------------------------===// // ObjCClassDecl //===----------------------------------------------------------------------===// diff --git a/lib/Sema/SemaDeclObjC.cpp b/lib/Sema/SemaDeclObjC.cpp index 1632e092e3..3d1f5bd064 100644 --- a/lib/Sema/SemaDeclObjC.cpp +++ b/lib/Sema/SemaDeclObjC.cpp @@ -377,18 +377,16 @@ ActOnStartClassInterface(SourceLocation AtInterfaceLoc, return ActOnObjCContainerStartDefinition(IDecl); } else { IDecl->setLocation(ClassLoc); - IDecl->setForwardDecl(false); IDecl->setAtStartLoc(AtInterfaceLoc); - // If the forward decl was in a PCH, we need to write it again in a - // dependent AST file. - IDecl->setChangedSinceDeserialization(true); // Since this ObjCInterfaceDecl was created by a forward declaration, // we now add it to the DeclContext since it wasn't added before // (see ActOnForwardClassDeclaration). IDecl->setLexicalDeclContext(CurContext); CurContext->addDecl(IDecl); - + + IDecl->completedForwardDecl(); + if (AttrList) ProcessDeclAttributeList(TUScope, IDecl, AttrList); } @@ -588,19 +586,16 @@ Sema::ActOnStartProtocolInterface(SourceLocation AtProtoInterfaceLoc, // Make sure the cached decl gets a valid start location. PDecl->setAtStartLoc(AtProtoInterfaceLoc); PDecl->setLocation(ProtocolLoc); - PDecl->setForwardDecl(false); // Since this ObjCProtocolDecl was created by a forward declaration, // we now add it to the DeclContext since it wasn't added before PDecl->setLexicalDeclContext(CurContext); CurContext->addDecl(PDecl); - // Repeat in dependent AST files. - PDecl->setChangedSinceDeserialization(true); + PDecl->completedForwardDecl(); } else { PDecl = ObjCProtocolDecl::Create(Context, CurContext, ProtocolName, ProtocolLoc, AtProtoInterfaceLoc, /*isForwardDecl=*/false); PushOnScopeChains(PDecl, TUScope); - PDecl->setForwardDecl(false); } if (AttrList) ProcessDeclAttributeList(TUScope, PDecl, AttrList); @@ -925,7 +920,8 @@ Decl *Sema::ActOnStartClassImplementation( // Mark the interface as being completed, even if it was just as // @class ....; // declaration; the user cannot reopen it. - IDecl->setForwardDecl(false); + if (IDecl->isForwardDecl()) + IDecl->completedForwardDecl(); } ObjCImplementationDecl* IMPDecl = diff --git a/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp index d414c07290..729cde0abf 100644 --- a/lib/Serialization/ASTReaderDecl.cpp +++ b/lib/Serialization/ASTReaderDecl.cpp @@ -558,7 +558,7 @@ void ASTDeclReader::VisitObjCInterfaceDecl(ObjCInterfaceDecl *ID) { // We will rebuild this list lazily. ID->setIvarList(0); ID->InitiallyForwardDecl = Record[Idx++]; - ID->setForwardDecl(Record[Idx++]); + ID->ForwardDecl = Record[Idx++]; ID->setImplicitInterfaceDecl(Record[Idx++]); ID->setSuperClassLoc(ReadSourceLocation(Record, Idx)); ID->setLocEnd(ReadSourceLocation(Record, Idx)); @@ -576,7 +576,7 @@ void ASTDeclReader::VisitObjCIvarDecl(ObjCIvarDecl *IVD) { void ASTDeclReader::VisitObjCProtocolDecl(ObjCProtocolDecl *PD) { VisitObjCContainerDecl(PD); PD->InitiallyForwardDecl = Record[Idx++]; - PD->setForwardDecl(Record[Idx++]); + PD->isForwardProtoDecl = Record[Idx++]; PD->setLocEnd(ReadSourceLocation(Record, Idx)); unsigned NumProtoRefs = Record[Idx++]; SmallVector ProtoRefs; diff --git a/lib/Serialization/ASTWriter.cpp b/lib/Serialization/ASTWriter.cpp index deaec02271..a98c4cc82b 100644 --- a/lib/Serialization/ASTWriter.cpp +++ b/lib/Serialization/ASTWriter.cpp @@ -2995,7 +2995,6 @@ void ASTWriter::WriteASTCore(Sema &SemaRef, MemorizeStatCalls *StatCalls, // Resolve any declaration pointers within the declaration updates block and // chained Objective-C categories block to declaration IDs. ResolveDeclUpdatesBlocks(); - ResolveChainedObjCCategories(); // Form the record of special types. RecordData SpecialTypes; @@ -3183,7 +3182,7 @@ void ASTWriter::ResolveDeclUpdatesBlocks() { const Decl *D = I->first; UpdateRecord &URec = I->second; - if (DeclsToRewrite.count(D)) + if (isRewritten(D)) continue; // The decl will be written completely unsigned Idx = 0, N = URec.size(); @@ -3216,7 +3215,7 @@ void ASTWriter::WriteDeclUpdatesBlocks() { const Decl *D = I->first; UpdateRecord &URec = I->second; - if (DeclsToRewrite.count(D)) + if (isRewritten(D)) continue; // The decl will be written completely,no need to store updates. uint64_t Offset = Stream.GetCurrentBitNo(); @@ -3243,17 +3242,6 @@ void ASTWriter::WriteDeclReplacementsBlock() { Stream.EmitRecord(DECL_REPLACEMENTS, Record); } -void ASTWriter::ResolveChainedObjCCategories() { - for (SmallVector::iterator - I = LocalChainedObjCCategories.begin(), - E = LocalChainedObjCCategories.end(); I != E; ++I) { - ChainedObjCCategoriesData &Data = *I; - Data.InterfaceID = GetDeclRef(Data.Interface); - Data.TailCategoryID = GetDeclRef(Data.TailCategory); - } - -} - void ASTWriter::WriteChainedObjCCategories() { if (LocalChainedObjCCategories.empty()) return; @@ -3263,13 +3251,16 @@ void ASTWriter::WriteChainedObjCCategories() { I = LocalChainedObjCCategories.begin(), E = LocalChainedObjCCategories.end(); I != E; ++I) { ChainedObjCCategoriesData &Data = *I; + if (isRewritten(Data.Interface)) + continue; + serialization::DeclID HeadCatID = getDeclID(Data.Interface->getCategoryList()); assert(HeadCatID != 0 && "Category not written ?"); - Record.push_back(Data.InterfaceID); + Record.push_back(GetDeclRef(Data.Interface)); Record.push_back(HeadCatID); - Record.push_back(Data.TailCategoryID); + Record.push_back(GetDeclRef(Data.TailCategory)); } Stream.EmitRecord(OBJC_CHAINED_CATEGORIES, Record); } @@ -4136,3 +4127,11 @@ void ASTWriter::AddedObjCCategoryToInterface(const ObjCCategoryDecl *CatD, ChainedObjCCategoriesData Data = { IFD, CatD, 0, 0 }; LocalChainedObjCCategories.push_back(Data); } + +void ASTWriter::CompletedObjCForwardRef(const ObjCContainerDecl *D) { + assert(!WritingAST && "Already writing the AST!"); + if (!D->isFromASTFile()) + return; // Declaration not imported from PCH. + + RewriteDecl(D); +} diff --git a/test/PCH/chain-categories2.m b/test/PCH/chain-categories2.m new file mode 100644 index 0000000000..dcff3d4af0 --- /dev/null +++ b/test/PCH/chain-categories2.m @@ -0,0 +1,44 @@ +// Test that infinite loop in rdar://10418538 was fixed. + +// Without PCH +// RUN: %clang_cc1 -fsyntax-only -verify -include %s -include %s %s + +// With PCH +// RUN: %clang_cc1 -fsyntax-only -verify %s -chain-include %s -chain-include %s + +#ifndef HEADER1 +#define HEADER1 +//===----------------------------------------------------------------------===// +// Primary header + +@class I; + +//===----------------------------------------------------------------------===// +#elif !defined(HEADER2) +#define HEADER2 +#if !defined(HEADER1) +#error Header inclusion order messed up +#endif + +//===----------------------------------------------------------------------===// +// Dependent header + +@interface I +@end + +@interface I(Cat1) +@end + +@interface I(Cat2) +@end + +//===----------------------------------------------------------------------===// +#else +//===----------------------------------------------------------------------===// + +void f(I* i) { + [i meth]; // expected-warning {{not found}} +} + +//===----------------------------------------------------------------------===// +#endif