[PATCH] PR23175 (fix) - Infinite loop iterating Objective-C method declarations in categories when the AST was deserialized from an .ast file

[Tom Honermann]

> Please add a testcase (I'd imagine it's not too hard to craft one in this case?).


I had posted the test case separately.  You can find it in http://reviews.llvm.org/D9126.  I had to craft a unit test.  I couldn't find a way to exploit the problem from just the Clang command line.


REPOSITORY
  rL LLVM

================
Comment at: lib/AST/DeclObjC.cpp:1745
@@ -1745,1 +1744,3 @@
+    if (ObjCCategoryDecl *CD =
+        IFace->FindCategoryDeclaration(ImplD->getIdentifier())) {
       Ctx.setObjCImplementation(CD, ImplD);
----------------
rsmith wrote:
> tahonermann wrote:
> > The subtle fix.  Calling ImplD->getIdentifier() is required to get the category name.  this->getIdentifier() returns the class name.
> Ouch. Can I tempt you to work on fixing the hiding of `getIdentifier` after this patch lands? :)
Oh, I was already tempted.  I'll try and find some time to fix it.  I'll be surprised if there aren't other cases like this waiting to be found.

================
Comment at: lib/Serialization/ASTReaderDecl.cpp:984-985
@@ -983,3 +983,4 @@
   VisitObjCContainerDecl(D);
-  D->setClassInterface(ReadDeclAs<ObjCInterfaceDecl>(Record, Idx));
+  // For ObjCCategoryImplDecl, setClassInterface must not be called until the
+  // category identifier has been deserialized and set.  See PR23175.
 }
----------------
rsmith wrote:
> Don't add PR references to the code; your comments should be complete enough to explain why we're doing what we're doing now without reference to what we got wrong before. In this case, I think they are. (People interested in the code's history can check the SVN logs.)
Funny.  Usually I'm the one telling people not to add such references.  Would you like me to repost the patch with the PR reference removed?

http://reviews.llvm.org/D9127

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/