dexonsmith added inline comments.
================
Comment at: clang/include/clang/Sema/Sema.h:1434-1436
+ bool addMethod(ObjCMethodDecl *Method) {
+ return AddedMethods.insert(Method).second;
+ }
----------------
rmaz wrote:
> 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.
>
That seems reasonable to me.
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
