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

Reply via email to