rmaz added inline comments.
================ Comment at: clang/include/clang/Sema/Sema.h:1434-1436 + bool addMethod(ObjCMethodDecl *Method) { + return AddedMethods.insert(Method).second; + } ---------------- rmaz wrote: > dexonsmith wrote: > > Hmm, I was imagining that the set would be more encapsulated than this, not > > just stored in the same place. > > > > I'm wondering if the following could be done in a prep commit: > > > > - Change Sema::addMethodToGlobalList to a private member function of > > GlobalMethodPool. > > - Make GlobalMethodPool::insert private > > - Add `GlobalMethodPool::addMethod(ObjCMethodDecl*,bool,bool)`, which does > > the second half of Sema::AddMethodToGlobalPool (the parts that don't need > > Sema's other fields), and change the latter to call the former. > > > > WDYT? > Definitely neater, will take a look at this later today. This might need a slightly different approach, as for the insert use case we have: ```lang=cxx Sema &S = *getSema(); Sema::GlobalMethodPool::iterator Pos = S.MethodPool.insert(std::make_pair(Sel, Sema::GlobalMethodPool::Lists())) .first; Pos->second.first.setBits(Visitor.getInstanceBits()); Pos->second.first.setHasMoreThanOneDecl(Visitor.instanceHasMoreThanOneDecl()); Pos->second.second.setBits(Visitor.getFactoryBits()); Pos->second.second.setHasMoreThanOneDecl(Visitor.factoryHasMoreThanOneDecl()); // Add methods to the global pool *after* setting hasMoreThanOneDecl, since // when building a module we keep every method individually and may need to // update hasMoreThanOneDecl as we add the methods. addMethodsToPool(S, Visitor.getInstanceMethods(), Pos->second.first); addMethodsToPool(S, Visitor.getFactoryMethods(), Pos->second.second); ``` At the moment we fetch a method list, modify its state and then start inserting the methods. If we move to something like `MethodPool.addMethod(ObjCMethodDecl *)` we will have to look up the method list for each insert, and we would need extra methods to set the state first on the method list. How about something like `MethodPool.addMethods(ArrayRef<ObjCMethodDecl*> methods, unsigned instanceBits, bool hasMoreThanOneDecl)`? Then we only need two list lookups per selector as before and we can handle the list state update before insert. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109632/new/ https://reviews.llvm.org/D109632 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits