[sema][objc] Minor refactor to OverrideSearch. NFCI.

[mattd]

Summary

• Removed a member that was only used during construction.
• Use range-based for iteration when accessing the result of the search.
• Require an ObjCMethodDecl reference upon construction of an
• Constify.

[rjmccall]

clang/lib/Sema/SemaDeclObjC.cpp

4168 ↗

(On Diff #195670)

We've been trying to move towards consistently passing around AST nodes as pointers, not references. Well, at least I am; please keep this as a pointer, at any rate.

4254 ↗

(On Diff #195670)

You could use auto here, I think the return type is obvious enough from the method.

4279 ↗

(On Diff #195670)

This could also be auto.

[mattd]

Thanks for the review, I addressed your comments:

• Removed the reference, and pass the AST node as a pointer.
• Made a few decls auto

[rjmccall]

LGTM.

lib/Sema/SemaDeclObjC.cpp

CheckRelatedResultTypeCompatibility(Sema &S, ObjCMethodDecl *Method,
   return Sema::RTC_Incompatible;
 }
 
 namespace {
 /// A helper class for searching for methods which a particular method
 /// overrides.
 class OverrideSearch {
 public:
-  Sema &S;
-  ObjCMethodDecl *Method;
+  const ObjCMethodDecl *Method;
   llvm::SmallSetVector<ObjCMethodDecl*, 4> Overridden;
   bool Recursive;
 
 public:
-  OverrideSearch(Sema &S, ObjCMethodDecl *method) : S(S), Method(method) {
+  OverrideSearch(Sema &S, const ObjCMethodDecl *method) : Method(method) {
     Selector selector = method->getSelector();
 
     // Bypass this search if we've never seen an instance/class method
     // with this selector before.
     Sema::GlobalMethodPool::iterator it = S.MethodPool.find(selector);
     if (it == S.MethodPool.end()) {
       if (!S.getExternalSource()) return;
       S.ReadMethodPool(selector);
 
       it = S.MethodPool.find(selector);
       if (it == S.MethodPool.end())
         return;
     }
-    ObjCMethodList &list =
+    const ObjCMethodList &list =
       method->isInstanceMethod() ? it->second.first : it->second.second;
     if (!list.getMethod()) return;
 
-    ObjCContainerDecl *container
+    const ObjCContainerDecl *container
       = cast<ObjCContainerDecl>(method->getDeclContext());
 
     // Prevent the search from reaching this container again.  This is
     // important with categories, which override methods from the
     // interface and each other.
-    if (ObjCCategoryDecl *Category = dyn_cast<ObjCCategoryDecl>(container)) {
+    if (const ObjCCategoryDecl *Category =
+            dyn_cast<ObjCCategoryDecl>(container)) {
       searchFromContainer(container);
-      if (ObjCInterfaceDecl *Interface = Category->getClassInterface())
+      if (const ObjCInterfaceDecl *Interface = Category->getClassInterface())
         searchFromContainer(Interface);
     } else {
       searchFromContainer(container);
     }
   }
 
   typedef decltype(Overridden)::iterator iterator;
   iterator begin() const { return Overridden.begin(); }
   iterator end() const { return Overridden.end(); }
 
 private:
-  void searchFromContainer(ObjCContainerDecl *container) {
+  void searchFromContainer(const ObjCContainerDecl *container) {
     if (container->isInvalidDecl()) return;
 
     switch (container->getDeclKind()) {
 #define OBJCCONTAINER(type, base) \
     case Decl::type: \
       searchFrom(cast<type##Decl>(container)); \
       break;
 #define ABSTRACT_DECL(expansion)
 #define DECL(type, base) \
     case Decl::type:
 #include "clang/AST/DeclNodes.inc"
       llvm_unreachable("not an ObjC container!");
     }
   }
 
-  void searchFrom(ObjCProtocolDecl *protocol) {
+  void searchFrom(const ObjCProtocolDecl *protocol) {
     if (!protocol->hasDefinition())
       return;
 
     // A method in a protocol declaration overrides declarations from
     // referenced ("parent") protocols.
     search(protocol->getReferencedProtocols());
   }
 
-  void searchFrom(ObjCCategoryDecl *category) {
+  void searchFrom(const ObjCCategoryDecl *category) {
     // A method in a category declaration overrides declarations from
     // the main class and from protocols the category references.
     // The main class is handled in the constructor.
     search(category->getReferencedProtocols());
   }
 
-  void searchFrom(ObjCCategoryImplDecl *impl) {
+  void searchFrom(const ObjCCategoryImplDecl *impl) {
     // A method in a category definition that has a category
     // declaration overrides declarations from the category
     // declaration.
     if (ObjCCategoryDecl *category = impl->getCategoryDecl()) {
       search(category);
       if (ObjCInterfaceDecl *Interface = category->getClassInterface())
         search(Interface);
 
     // Otherwise it overrides declarations from the class.
-    } else if (ObjCInterfaceDecl *Interface = impl->getClassInterface()) {
+    } else if (const auto *Interface = impl->getClassInterface()) {
       search(Interface);
     }
   }
 
-  void searchFrom(ObjCInterfaceDecl *iface) {
+  void searchFrom(const ObjCInterfaceDecl *iface) {
     // A method in a class declaration overrides declarations from
     if (!iface->hasDefinition())
       return;
 
     //   - categories,
     for (auto *Cat : iface->known_categories())
       search(Cat);
 
     //   - the super class, and
     if (ObjCInterfaceDecl *super = iface->getSuperClass())
       search(super);
 
     //   - any referenced protocols.
     search(iface->getReferencedProtocols());
   }
 
-  void searchFrom(ObjCImplementationDecl *impl) {
+  void searchFrom(const ObjCImplementationDecl *impl) {
     // A method in a class implementation overrides declarations from
     // the class interface.
-    if (ObjCInterfaceDecl *Interface = impl->getClassInterface())
+    if (const auto *Interface = impl->getClassInterface())
       search(Interface);
   }
 
   void search(const ObjCProtocolList &protocols) {
-    for (ObjCProtocolList::iterator i = protocols.begin(), e = protocols.end();
-         i != e; ++i)
-      search(*i);
+    for (const auto *Proto : protocols)
+      search(Proto);
   }
 
-  void search(ObjCContainerDecl *container) {
+  void search(const ObjCContainerDecl *container) {
     // Check for a method in this container which matches this selector.
     ObjCMethodDecl *meth = container->getMethod(Method->getSelector(),
                                                 Method->isInstanceMethod(),
                                                 /*AllowHidden=*/true);
 
     // If we find one, record it and bail out.
     if (meth) {
       Overridden.insert(meth);
void search(const ObjCContainerDecl *container) {
     searchFromContainer(container);
   }
 };
 } // end anonymous namespace
 
 void Sema::CheckObjCMethodOverrides(ObjCMethodDecl *ObjCMethod,
                                     ObjCInterfaceDecl *CurrentClass,
                                     ResultTypeCompatibilityKind RTC) {
+  if (!ObjCMethod)
+    return;
   // Search for overridden methods and merge information down from them.
   OverrideSearch overrides(*this, ObjCMethod);
   // Keep track if the method overrides any method in the class's base classes,
   // its protocols, or its categories' protocols; we will keep that info
   // in the ObjCMethodDecl.
   // For this info, a method in an implementation is not considered as
   // overriding the same method in the interface or its categories.
   bool hasOverriddenMethodsInBaseOrProtocol = false;
-  for (OverrideSearch::iterator
-         i = overrides.begin(), e = overrides.end(); i != e; ++i) {
-    ObjCMethodDecl *overridden = *i;
-
+  for (ObjCMethodDecl *overridden : overrides) {
     if (!hasOverriddenMethodsInBaseOrProtocol) {
       if (isa<ObjCProtocolDecl>(overridden->getDeclContext()) ||
           CurrentClass != overridden->getClassInterface() ||
           overridden->isOverriding()) {
         hasOverriddenMethodsInBaseOrProtocol = true;
 
       } else if (isa<ObjCImplDecl>(ObjCMethod->getDeclContext())) {
         // OverrideSearch will return as "overridden" the same method in the
if (!hasOverriddenMethodsInBaseOrProtocol) {
             ObjCMethod->isInstanceMethod()? It->second.first: It->second.second;
           unsigned CategCount = List.getBits();
           if (CategCount > 0) {
             // If the method is in a category we'll do lookup if there were at
             // least 2 category methods recorded, otherwise only one will do.
             if (CategCount > 1 ||
                 !isa<ObjCCategoryImplDecl>(overridden->getDeclContext())) {
               OverrideSearch overrides(*this, overridden);
-              for (OverrideSearch::iterator
-                     OI= overrides.begin(), OE= overrides.end(); OI!=OE; ++OI) {
-                ObjCMethodDecl *SuperOverridden = *OI;
+              for (ObjCMethodDecl *SuperOverridden : overrides) {
                 if (isa<ObjCProtocolDecl>(SuperOverridden->getDeclContext()) ||
                     CurrentClass != SuperOverridden->getClassInterface()) {
                   hasOverriddenMethodsInBaseOrProtocol = true;
                   overridden->setOverriding(true);
                   break;
                 }
               }
             }