> 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/ _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
