dexonsmith added inline comments.
================ Comment at: clang/include/clang/Sema/Sema.h:1423-1426 + ObjCMethodList InstanceMethods; + ObjCMethodList FactoryMethods; + llvm::DenseSet<ObjCMethodDecl *> AddedInstanceMethods; + llvm::DenseSet<ObjCMethodDecl *> AddedFactoryMethods; ---------------- Do these two sets really need to be per-selector (per `GlobalMethods` object in GlobalMethodPool)? I feel like they could be global to `Sema`, as long as the same `ObjCMethodDecl` is never added to GlobalMethodPool for more than one selector. (It seems plausible we have that property since presumably ObjCMethodDecl::getSelector is always the key used for inserting into GlobalMethodPool below... can you confirm?) Assuming you can pull the sets out to represent the GlobalMethodPool as a whole, then I suggest creating a data structure for GlobalMethodPool that encapsulates the DenseMap and the two sets (maybe: rename "GlobalMethods" to "GlobalMethodLists", rename "GlobalMethodPool" to "GlobalMethods", and give the new data structure the name "GlobalMethodPool"). Best to do it as multiple commits: NFC commit(s) to refactor (renames, create new type update call sites to use new APIs, etc.), and a final commit that changes the functionality (adding the set behaviour) but that doesn't need to touch call sites. On the other hand, if the sets really need to be per-selector (please explain why if so...), please use SmallPtrSet here instead of DenseSet to avoid allocation in the common case of 1 decl per selector. I'd suggest encapsulating the list/set together somehow (maybe starting with an NFC refactoring to add a "list/set" data structure at the top-level (maybe rename ObjCMethodList => ObjCMethodListNode, and the new list has a private member called "Head" and necessary APIs for insertion/etc), then in a second commit adding the SmallPtrSet into the list data structure and use it to gate the "insert" operation). 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
