[cfe-commits] r144465 - in /cfe/trunk: include/clang/AST/ASTMutationListener.h include/clang/AST/DeclObjC.h include/clang/Serialization/ASTWriter.h lib/AST/ASTImporter.cpp lib/AST/DeclObjC.cpp lib/Sema/SemaDeclObjC.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriter.cpp test/PCH/chain-categories2.m

Argyrios Kyrtzidis akyrtzi at gmail.com
Sat Nov 12 13:07:47 PST 2011


Author: akirtzidis
Date: Sat Nov 12 15:07:46 2011
New Revision: 144465

URL: http://llvm.org/viewvc/llvm-project?rev=144465&view=rev
Log:
[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.

Added:
    cfe/trunk/test/PCH/chain-categories2.m
Modified:
    cfe/trunk/include/clang/AST/ASTMutationListener.h
    cfe/trunk/include/clang/AST/DeclObjC.h
    cfe/trunk/include/clang/Serialization/ASTWriter.h
    cfe/trunk/lib/AST/ASTImporter.cpp
    cfe/trunk/lib/AST/DeclObjC.cpp
    cfe/trunk/lib/Sema/SemaDeclObjC.cpp
    cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
    cfe/trunk/lib/Serialization/ASTWriter.cpp

Modified: cfe/trunk/include/clang/AST/ASTMutationListener.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTMutationListener.h?rev=144465&r1=144464&r2=144465&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ASTMutationListener.h (original)
+++ cfe/trunk/include/clang/AST/ASTMutationListener.h Sat Nov 12 15:07:46 2011
@@ -24,6 +24,7 @@
   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 @@
   /// \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

Modified: cfe/trunk/include/clang/AST/DeclObjC.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=144465&r1=144464&r2=144465&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclObjC.h (original)
+++ cfe/trunk/include/clang/AST/DeclObjC.h Sat Nov 12 15:07:46 2011
@@ -710,7 +710,8 @@
   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 @@
   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

Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=144465&r1=144464&r2=144465&view=diff
==============================================================================
--- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTWriter.h Sat Nov 12 15:07:46 2011
@@ -394,7 +394,6 @@
   void ResolveDeclUpdatesBlocks();
   void WriteDeclUpdatesBlocks();
   void WriteDeclReplacementsBlock();
-  void ResolveChainedObjCCategories();
   void WriteChainedObjCCategories();
   void WriteDeclContextVisibleUpdate(const DeclContext *DC);
   void WriteFPPragmaOptions(const FPOptions &Opts);
@@ -592,6 +591,10 @@
     const_cast<Decl *>(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 @@
   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

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=144465&r1=144464&r2=144465&view=diff
==============================================================================
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Sat Nov 12 15:07:46 2011
@@ -3111,9 +3111,10 @@
                                          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 @@
       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);
 

Modified: cfe/trunk/lib/AST/DeclObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclObjC.cpp?rev=144465&r1=144464&r2=144465&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclObjC.cpp (original)
+++ cfe/trunk/lib/AST/DeclObjC.cpp Sat Nov 12 15:07:46 2011
@@ -217,6 +217,13 @@
   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 @@
   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
 //===----------------------------------------------------------------------===//

Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=144465&r1=144464&r2=144465&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Sat Nov 12 15:07:46 2011
@@ -377,18 +377,16 @@
       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 @@
     // 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 @@
     // 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 =

Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=144465&r1=144464&r2=144465&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Sat Nov 12 15:07:46 2011
@@ -558,7 +558,7 @@
   // 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::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<ObjCProtocolDecl *, 16> ProtoRefs;

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=144465&r1=144464&r2=144465&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Sat Nov 12 15:07:46 2011
@@ -2995,7 +2995,6 @@
   // 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 @@
     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 @@
     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 @@
   Stream.EmitRecord(DECL_REPLACEMENTS, Record);
 }
 
-void ASTWriter::ResolveChainedObjCCategories() {
-  for (SmallVector<ChainedObjCCategoriesData, 16>::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 @@
          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 @@
   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);
+}

Added: cfe/trunk/test/PCH/chain-categories2.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/chain-categories2.m?rev=144465&view=auto
==============================================================================
--- cfe/trunk/test/PCH/chain-categories2.m (added)
+++ cfe/trunk/test/PCH/chain-categories2.m Sat Nov 12 15:07:46 2011
@@ -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
+
+ at class I;
+
+//===----------------------------------------------------------------------===//
+#elif !defined(HEADER2)
+#define HEADER2
+#if !defined(HEADER1)
+#error Header inclusion order messed up
+#endif
+
+//===----------------------------------------------------------------------===//
+// Dependent header
+
+ at interface I
+ at end
+
+ at interface I(Cat1)
+ at end
+
+ at interface I(Cat2)
+ at end
+
+//===----------------------------------------------------------------------===//
+#else
+//===----------------------------------------------------------------------===//
+
+void f(I* i) {
+  [i meth]; // expected-warning {{not found}}
+}
+
+//===----------------------------------------------------------------------===//
+#endif





More information about the cfe-commits mailing list