[clang][NFC] encapsulate global method list in GlobalMethodPool

[rmaz]

Summary

This change moves the addMethodToGlobalList function to be a
private member function of the GlobalMethodPool class. This
is a preparatory step to allow for de-duplication of inserted
methods.

Two public methods are added to handle the existing use cases
for adding methods to a global list: addMethodsForSelector
and addMethod. The former is required to avoid the overhead
of looking up the Methods map for each insert when adding
multiple methods for a single selector when deserializing AST
files. It also allows for modifying the list properties first.

[rmaz]

remove unnecessary empty check

[rmaz]

Abandoning in favor of D110123

clang/include/clang/Sema/Sema.h

 
   /// Retrieves list of suspicious delete-expressions that will be checked at
   /// the end of translation unit.
   const llvm::MapVector<FieldDecl *, DeleteLocs> &
   getMismatchingDeleteExpressions() const;
 
   class GlobalMethodPool {
   public:
+    GlobalMethodPool(Sema &S) : SemaRef(S) {}
+    GlobalMethodPool() = delete;
     using Lists = std::pair<ObjCMethodList, ObjCMethodList>;
     using iterator = llvm::DenseMap<Selector, Lists>::iterator;
     iterator begin() { return Methods.begin(); }
     iterator end() { return Methods.end(); }
     iterator find(Selector Sel) { return Methods.find(Sel); }
-    std::pair<iterator, bool> insert(std::pair<Selector, Lists> &&Val) {
-      return Methods.insert(Val);
-    }
     int count(Selector Sel) const { return Methods.count(Sel); }
     bool empty() const { return Methods.empty(); }
 
+    void addMethodsForSelector(Selector Sel, ArrayRef<ObjCMethodDecl *> Decls,
+                               bool Instance, unsigned Bits,
+                               bool HasMoreThanOneDecl) {
+      auto Pos = Methods.insert(std::make_pair(Sel, Lists())).first;
+      auto &List = Instance ? Pos->second.first : Pos->second.second;
+      List.setBits(Bits);
+      List.setHasMoreThanOneDecl(HasMoreThanOneDecl);
+
+      // Add methods to the global list *after* setting hasMoreThanOneDecl,
+      // since when building a module we keep every method individually and may
+      // need to update hasMoreThanOneDecl as we add the methods.
+      for (auto *D : Decls) {
+        addMethodToGlobalList(&List, D);
+      }
+    }
+
+    void addMethod(ObjCMethodDecl *Method) {
+      auto Pos = Methods.find(Method->getSelector());
+      if (Pos == Methods.end()) {
+        Pos = Methods.insert(std::make_pair(Method->getSelector(), Lists()))
+                  .first;
+      }
+
+      auto &List =
+          Method->isInstanceMethod() ? Pos->second.first : Pos->second.second;
+      addMethodToGlobalList(&List, Method);
+    }
+
   private:
+    Sema &SemaRef;
     llvm::DenseMap<Selector, Lists> Methods;
+    void addMethodToGlobalList(ObjCMethodList *List, ObjCMethodDecl *Method);
   };
 
   /// Method Pool - allows efficient lookup when typechecking messages to "id".
   /// We need to maintain a list, since selectors can have differing signatures
   /// across classes. In Cocoa, this happens to be extremely uncommon (only 1%
   /// of selectors are "overloaded").
   /// At the head of the list it is recorded whether there were 0, 1, or >= 2
   /// methods inside categories with a particular selector.
void MatchAllMethodDeclarations(const SelectorSet &InsMap,
                                   bool ImmediateClass,
                                   bool WarnCategoryMethodImpl=false);
 
   /// CheckCategoryVsClassMethodMatches - Checks that methods implemented in
   /// category matches with those implemented in its primary class and
   /// warns each time an exact match is found.
   void CheckCategoryVsClassMethodMatches(ObjCCategoryImplDecl *CatIMP);
 
-  /// Add the given method to the list of globally-known methods.
-  void addMethodToGlobalList(ObjCMethodList *List, ObjCMethodDecl *Method);
-
   /// Returns default addr space for method qualifiers.
   LangAS getDefaultCXXMethodAddrSpace() const;
 
 private:
   /// AddMethodToGlobalPool - Add an instance or factory method to the global
   /// pool. See descriptoin of AddInstanceMethodToGlobalPool.
   void AddMethodToGlobalPool(ObjCMethodDecl *Method, bool impl, bool instance);
 

clang/lib/Sema/Sema.cpp

: ExternalSource(nullptr), isMultiplexExternalSource(false),
       LateTemplateParserCleanup(nullptr), OpaqueParser(nullptr), IdResolver(pp),
       StdExperimentalNamespaceCache(nullptr), StdInitializerList(nullptr),
       StdCoroutineTraitsCache(nullptr), CXXTypeInfoDecl(nullptr),
       MSVCGuidDecl(nullptr), NSNumberDecl(nullptr), NSValueDecl(nullptr),
       NSStringDecl(nullptr), StringWithUTF8StringMethod(nullptr),
       ValueWithBytesObjCTypeMethod(nullptr), NSArrayDecl(nullptr),
       ArrayWithObjectsMethod(nullptr), NSDictionaryDecl(nullptr),
       DictionaryWithObjectsMethod(nullptr), GlobalNewDeleteDeclared(false),
-      TUKind(TUKind), NumSFINAEErrors(0),
+      TUKind(TUKind), NumSFINAEErrors(0), MethodPool(*this),
       FullyCheckedComparisonCategories(
           static_cast<unsigned>(ComparisonCategoryType::Last) + 1),
       SatisfactionCache(Context), AccessCheckingSFINAE(false),
       InNonInstantiationSFINAEContext(false), NonInstantiationEntries(0),
       ArgumentPackSubstitutionIndex(-1), CurrentInstantiationScope(nullptr),
       DisableTypoCorrection(false), TyposCorrected(0), AnalysisWarnings(*this),
       ThreadSafetyDeclCache(nullptr), VarDataSharingAttributesStack(nullptr),
       CurScope(nullptr), Ident_super(nullptr), Ident___float128(nullptr) {

clang/lib/Sema/SemaDeclObjC.cpp

if (MethodProtocol && MethodInListProtocol)
     return true;
 
   ObjCInterfaceDecl *MethodInterface = Method->getClassInterface();
   ObjCInterfaceDecl *MethodInListInterface =
       MethodInList->getClassInterface();
   return MethodInterface == MethodInListInterface;
 }
 
-void Sema::addMethodToGlobalList(ObjCMethodList *List,
+void Sema::GlobalMethodPool::addMethodToGlobalList(ObjCMethodList *List,
                                                    ObjCMethodDecl *Method) {
   // Record at the head of the list whether there were 0, 1, or >= 2 methods
   // inside categories.
   if (ObjCCategoryDecl *CD =
           dyn_cast<ObjCCategoryDecl>(Method->getDeclContext()))
     if (!CD->IsClassExtension() && List->getBits() < 2)
       List->setBits(List->getBits() + 1);
 
   // If the list is empty, make it a singleton list.
   if (List->getMethod() == nullptr) {
     List->setMethod(Method);
     List->setNext(nullptr);
     return;
   }
 
   // We've seen a method with this name, see if we have already seen this type
   // signature.
   ObjCMethodList *Previous = List;
   ObjCMethodList *ListWithSameDeclaration = nullptr;
   for (; List; Previous = List, List = List->getNext()) {
     // If we are building a module, keep all of the methods.
-    if (getLangOpts().isCompilingModule())
+    if (SemaRef.getLangOpts().isCompilingModule())
       continue;
 
-    bool SameDeclaration = MatchTwoMethodDeclarations(Method,
-                                                      List->getMethod());
+    bool SameDeclaration =
+        SemaRef.MatchTwoMethodDeclarations(Method, List->getMethod());
     // Looking for method with a type bound requires the correct context exists.
     // We need to insert a method into the list if the context is different.
     // If the method's declaration matches the list
     // a> the method belongs to a different context: we need to insert it, in
     //    order to emit the availability message, we need to prioritize over
     //    availability among the methods with the same declaration.
     // b> the method belongs to the same context: there is no need to insert a
     //    new entry.
if (Method->isUnavailable()) {
         List->setMethod(Method);
     }
 
     return;
   }
 
   // We have a new signature for an existing method - add it.
   // This is extremely rare. Only 1% of Cocoa selectors are "overloaded".
-  ObjCMethodList *Mem = BumpAlloc.Allocate<ObjCMethodList>();
+  ObjCMethodList *Mem = SemaRef.BumpAlloc.Allocate<ObjCMethodList>();
 
   // We insert it right before ListWithSameDeclaration.
   if (ListWithSameDeclaration) {
     auto *List = new (Mem) ObjCMethodList(*ListWithSameDeclaration);
     // FIXME: should we clear the other bits in ListWithSameDeclaration?
     ListWithSameDeclaration->setMethod(Method);
     ListWithSameDeclaration->setNext(List);
     return;
void Sema::AddMethodToGlobalPool(ObjCMethodDecl *Method, bool impl,
                                  bool instance) {
   // Ignore methods of invalid containers.
   if (cast<Decl>(Method->getDeclContext())->isInvalidDecl())
     return;
 
   if (ExternalSource)
     ReadMethodPool(Method->getSelector());
 
-  GlobalMethodPool::iterator Pos = MethodPool.find(Method->getSelector());
-  if (Pos == MethodPool.end())
-    Pos = MethodPool
-              .insert(std::make_pair(Method->getSelector(),
-                                     GlobalMethodPool::Lists()))
-              .first;
-
   Method->setDefined(impl);
-
-  ObjCMethodList &Entry = instance ? Pos->second.first : Pos->second.second;
-  addMethodToGlobalList(&Entry, Method);
+  assert(Method->isInstanceMethod() == instance);
+  MethodPool.addMethod(Method);
 }
 
 /// Determines if this is an "acceptable" loose mismatch in the global
 /// method pool.  This exists mostly as a hack to get around certain
 /// global mismatches which we can't afford to make warnings / errors.
 /// Really, what we want is a way to take a method out of the global
 /// method pool.
 static bool isAcceptableMethodMismatch(ObjCMethodDecl *chosen,

clang/lib/Serialization/ASTReader.cpp

public:
     }
 
     bool factoryHasMoreThanOneDecl() const { return FactoryHasMoreThanOneDecl; }
   };
 
 } // namespace serialization
 } // namespace clang
 
-/// Add the given set of methods to the method list.
-static void addMethodsToPool(Sema &S, ArrayRef<ObjCMethodDecl *> Methods,
-                             ObjCMethodList &List) {
-  for (unsigned I = 0, N = Methods.size(); I != N; ++I) {
-    S.addMethodToGlobalList(&List, Methods[I]);
-  }
-}
-
 void ASTReader::ReadMethodPool(Selector Sel) {
   // Get the selector generation and update it to the current generation.
   unsigned &Generation = SelectorGeneration[Sel];
   unsigned PriorGeneration = Generation;
   Generation = getGeneration();
   SelectorOutOfDate[Sel] = false;
 
   // Search for methods defined with this selector.
   ++NumMethodPoolLookups;
   ReadMethodPoolVisitor Visitor(*this, Sel, PriorGeneration);
   ModuleMgr.visit(Visitor);
 
   if (Visitor.getInstanceMethods().empty() &&
       Visitor.getFactoryMethods().empty())
     return;
 
   ++NumMethodPoolHits;
 
   if (!getSema())
     return;
 
   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);
+  S.MethodPool.addMethodsForSelector(Sel, Visitor.getInstanceMethods(), true,
+                                     Visitor.getInstanceBits(),
+                                     Visitor.instanceHasMoreThanOneDecl());
+  S.MethodPool.addMethodsForSelector(Sel, Visitor.getFactoryMethods(), false,
+                                     Visitor.getFactoryBits(),
+                                     Visitor.factoryHasMoreThanOneDecl());
 }
 
 void ASTReader::updateOutOfDateSelector(Selector Sel) {
   if (SelectorOutOfDate[Sel])
     ReadMethodPool(Sel);
 }
 
 void ASTReader::ReadKnownNamespaces(