[ODRHash] Hash `ObjCMethodDecl` and diagnose discovered mismatches.

[vsapsai]

Use diagnoseSubMismatchMethodParameters both for Obj-C and C++ methods.

[vsapsai]

Rebase.

[jansvoboda11]

This look good to me, but you might want to get a review from someone who worked on this before.

[bruno]

LGTM

[vsapsai]

Thanks for the review! I'll wait for the blocking D130324 to land before I commit this change. I appreciate your speedy response.

clang/include/clang/AST/ODRDiagsEmitter.h

public:
 
   /// Get the best name we know for the module that owns the given
   /// declaration, or an empty string if the declaration is not from a module.
   static std::string getOwningModuleNameForDiagnostic(const Decl *D);
 
 private:
   using DeclHashes = llvm::SmallVector<std::pair<const Decl *, unsigned>, 4>;
 
-  // Used with err_module_odr_violation_mismatch_decl and
-  // note_module_odr_violation_mismatch_decl
+  // Used with err_module_odr_violation_mismatch_decl,
+  // note_module_odr_violation_mismatch_decl,
+  // err_module_odr_violation_mismatch_decl_unknown,
+  // and note_module_odr_violation_mismatch_decl_unknown
   // This list should be the same Decl's as in ODRHash::isSubDeclToBeProcessed
   enum ODRMismatchDecl {
     EndOfClass,
     PublicSpecifer,
     PrivateSpecifer,
     ProtectedSpecifer,
     StaticAssert,
     Field,
     CXXMethod,
     TypeAlias,
     TypeDef,
     Var,
     Friend,
     FunctionTemplate,
+    ObjCMethod,
     Other
   };
 
   struct DiffResult {
     const Decl *FirstDecl = nullptr, *SecondDecl = nullptr;
     ODRMismatchDecl FirstDiffType = Other, SecondDiffType = Other;
   };
 
private:
   /// Returns true if found a mismatch and diagnosed it.
   bool diagnoseSubMismatchProtocols(const ObjCProtocolList &FirstProtocols,
                                     const ObjCContainerDecl *FirstContainer,
                                     StringRef FirstModule,
                                     const ObjCProtocolList &SecondProtocols,
                                     const ObjCContainerDecl *SecondContainer,
                                     StringRef SecondModule) const;
 
+  /// Check if Objective-C methods are the same and diagnose if different.
+  ///
+  /// Returns true if found a mismatch and diagnosed it.
+  bool diagnoseSubMismatchObjCMethod(const NamedDecl *FirstObjCContainer,
+                                     StringRef FirstModule,
+                                     StringRef SecondModule,
+                                     const ObjCMethodDecl *FirstMethod,
+                                     const ObjCMethodDecl *SecondMethod) const;
+
 private:
   DiagnosticsEngine &Diags;
   const ASTContext &Context;
   const LangOptions &LangOpts;
 };
 
 } // namespace clang
 
 #endif

clang/include/clang/Basic/DiagnosticASTKinds.td

def note_module_odr_violation_template_parameter : Note <
   "template parameter with %select{no |}2default argument|"
   "template parameter with different default argument}1">;
 
 def err_module_odr_violation_mismatch_decl : Error<
   "%q0 has different definitions in different modules; first difference is "
   "%select{definition in module '%2'|defined here}1 found "
   "%select{end of class|public access specifier|private access specifier|"
   "protected access specifier|static assert|field|method|type alias|typedef|"
-  "data member|friend declaration|function template}3">;
+  "data member|friend declaration|function template|method}3">;
 def note_module_odr_violation_mismatch_decl : Note<"but in '%0' found "
   "%select{end of class|public access specifier|private access specifier|"
   "protected access specifier|static assert|field|method|type alias|typedef|"
-  "data member|friend declaration|function template}1">;
+  "data member|friend declaration|function template|method}1">;
 
 def err_module_odr_violation_record : Error<
   "%q0 has different definitions in different modules; first difference is "
   "%select{definition in module '%2'|defined here}1 found "
   "%select{"
   "static assert with condition|"
   "static assert with message|"
   "static assert with %select{|no }4message|"
   "%select{method %5|constructor|destructor}4|"
   "%select{method %5|constructor|destructor}4 "
     "is %select{not deleted|deleted}6|"
   "%select{method %5|constructor|destructor}4 "
     "is %select{not defaulted|defaulted}6|"
   "%select{method %5|constructor|destructor}4 "
     "is %select{|pure }6%select{not virtual|virtual}7|"
   "%select{method %5|constructor|destructor}4 "
     "is %select{not static|static}6|"
   "%select{method %5|constructor|destructor}4 "
     "is %select{not volatile|volatile}6|"
   "%select{method %5|constructor|destructor}4 "
     "is %select{not const|const}6|"
   "%select{method %5|constructor|destructor}4 "
     "is %select{not inline|inline}6|"
   "%select{method %5|constructor|destructor}4 "
-    "that has %6 parameter%s6|"
-  "%select{method %5|constructor|destructor}4 "
-    "with %ordinal6 parameter of type %7%select{| decayed from %9}8|"
-  "%select{method %5|constructor|destructor}4 "
-    "with %ordinal6 parameter named %7|"
-  "%select{method %5|constructor|destructor}4 "
     "with %ordinal6 parameter with%select{out|}7 a default argument|"
   "%select{method %5|constructor|destructor}4 "
     "with %ordinal6 parameter with a default argument|"
   "%select{method %5|constructor|destructor}4 "
     "with %select{no |}6template arguments|"
   "%select{method %5|constructor|destructor}4 "
     "with %6 template argument%s6|"
   "%select{method %5|constructor|destructor}4 "
"%select{method %3|constructor|destructor}2 "
     "is %select{not static|static}4|"
   "%select{method %3|constructor|destructor}2 "
     "is %select{not volatile|volatile}4|"
   "%select{method %3|constructor|destructor}2 "
     "is %select{not const|const}4|"
   "%select{method %3|constructor|destructor}2 "
     "is %select{not inline|inline}4|"
   "%select{method %3|constructor|destructor}2 "
-    "that has %4 parameter%s4|"
-  "%select{method %3|constructor|destructor}2 "
-    "with %ordinal4 parameter of type %5%select{| decayed from %7}6|"
-  "%select{method %3|constructor|destructor}2 "
-    "with %ordinal4 parameter named %5|"
-  "%select{method %3|constructor|destructor}2 "
     "with %ordinal4 parameter with%select{out|}5 a default argument|"
   "%select{method %3|constructor|destructor}2 "
     "with %ordinal4 parameter with a different default argument|"
   "%select{method %3|constructor|destructor}2 "
     "with %select{no |}4template arguments|"
   "%select{method %3|constructor|destructor}2 "
     "with %4 template argument%s4|"
   "%select{method %3|constructor|destructor}2 "
def err_module_odr_violation_referenced_protocols : Error <
   "%ordinal4 referenced protocol with name %5"
   "}3">;
 def note_module_odr_violation_referenced_protocols : Note <"but in '%0' found "
   "%select{"
   "%2 referenced %plural{1:protocol|:protocols}2|"
   "%ordinal2 referenced protocol with different name %3"
   "}1">;
 
+def err_module_odr_violation_objc_method : Error<
+  "%q0 has different definitions in different modules; first difference is "
+  "%select{definition in module '%2'|defined here}1 found "
+  "%select{"
+  "method %4 with return type %5|"
+  "%select{class|instance}5 method %4|"
+  "%select{no|'required'|'optional'}4 method control|"
+  "method %4 with %select{no designated initializer|designated initializer}5|"
+  "%select{regular|direct}5 method %4|"
+  "method %4"
+  "}3">;
+def note_module_odr_violation_objc_method : Note<"but in '%0' found "
+  "%select{"
+  "method %2 with different return type %3|"
+  "method %2 as %select{class|instance}3 method|"
+  "%select{no|'required'|'optional'}2 method control|"
+  "method %2 with %select{no designated initializer|designated initializer}3|"
+  "%select{regular|direct}3 method %2|"
+  "different method %2"
+  "}1">;
+
+def err_module_odr_violation_method_params : Error<
+  "%q0 has different definitions in different modules; first difference is "
+  "%select{definition in module '%2'|defined here}1 found "
+  "%select{"
+  "%select{method %5|constructor|destructor}4 "
+    "that has %6 parameter%s6|"
+  "%select{method %5|constructor|destructor}4 "
+    "with %ordinal6 parameter of type %7%select{| decayed from %9}8|"
+  "%select{method %5|constructor|destructor}4 "
+    "with %ordinal6 parameter named %7"
+  "}3">;
+def note_module_odr_violation_method_params : Note<"but in '%0' found "
+  "%select{"
+  "%select{method %3|constructor|destructor}2 "
+    "that has %4 parameter%s4|"
+  "%select{method %3|constructor|destructor}2 "
+    "with %ordinal4 parameter of type %5%select{| decayed from %7}6|"
+  "%select{method %3|constructor|destructor}2 "
+    "with %ordinal4 parameter named %5"
+  "}1">;
+
 def err_module_odr_violation_mismatch_decl_unknown : Error<
   "%q0 %select{with definition in module '%2'|defined here}1 has different "
   "definitions in different modules; first difference is this "
   "%select{||||static assert|field|method|type alias|typedef|data member|"
-  "friend declaration|function template|"
+  "friend declaration|function template|method|"
   "unexpected decl}3">;
 def note_module_odr_violation_mismatch_decl_unknown : Note<
   "but in '%0' found "
   "%select{||||different static assert|different field|different method|"
   "different type alias|different typedef|different data member|"
-  "different friend declaration|different function template|"
+  "different friend declaration|different function template|different method|"
   "another unexpected decl}1">;
 
 
 def remark_sanitize_address_insert_extra_padding_accepted : Remark<
     "-fsanitize-address-field-padding applied to %0">, ShowInSystemHeader,
     InGroup<SanitizeAddressRemarks>;
 def remark_sanitize_address_insert_extra_padding_rejected : Remark<
     "-fsanitize-address-field-padding ignored for %0 because it "

clang/lib/AST/ODRDiagsEmitter.cpp

std::string ODRDiagsEmitter::getOwningModuleNameForDiagnostic(const Decl *D) {
   // If we know the owning module, use it.
   if (Module *M = D->getImportedOwningModule())
     return M->getFullModuleName();
 
   // Not from a module.
   return {};
 }
 
+template <typename MethodT>
+static bool diagnoseSubMismatchMethodParameters(DiagnosticsEngine &Diags,
+                                                const NamedDecl *FirstContainer,
+                                                StringRef FirstModule,
+                                                StringRef SecondModule,
+                                                const MethodT *FirstMethod,
+                                                const MethodT *SecondMethod) {
+  enum DiagMethodType {
+    DiagMethod,
+    DiagConstructor,
+    DiagDestructor,
+  };
+  auto GetDiagMethodType = [](const NamedDecl *D) {
+    if (isa<CXXConstructorDecl>(D))
+      return DiagConstructor;
+    if (isa<CXXDestructorDecl>(D))
+      return DiagDestructor;
+    return DiagMethod;
+  };
+
+  enum ODRMethodParametersDifference {
+    NumberParameters,
+    ParameterType,
+    ParameterName,
+  };
+  auto DiagError = [&Diags, &GetDiagMethodType, FirstContainer, FirstModule,
+                    FirstMethod](ODRMethodParametersDifference DiffType) {
+    DeclarationName FirstName = FirstMethod->getDeclName();
+    DiagMethodType FirstMethodType = GetDiagMethodType(FirstMethod);
+    return Diags.Report(FirstMethod->getLocation(),
+                        diag::err_module_odr_violation_method_params)
+           << FirstContainer << FirstModule.empty() << FirstModule
+           << FirstMethod->getSourceRange() << DiffType << FirstMethodType
+           << FirstName;
+  };
+  auto DiagNote = [&Diags, &GetDiagMethodType, SecondModule,
+                   SecondMethod](ODRMethodParametersDifference DiffType) {
+    DeclarationName SecondName = SecondMethod->getDeclName();
+    DiagMethodType SecondMethodType = GetDiagMethodType(SecondMethod);
+    return Diags.Report(SecondMethod->getLocation(),
+                        diag::note_module_odr_violation_method_params)
+           << SecondModule << SecondMethod->getSourceRange() << DiffType
+           << SecondMethodType << SecondName;
+  };
+
+  const unsigned FirstNumParameters = FirstMethod->param_size();
+  const unsigned SecondNumParameters = SecondMethod->param_size();
+  if (FirstNumParameters != SecondNumParameters) {
+    DiagError(NumberParameters) << FirstNumParameters;
+    DiagNote(NumberParameters) << SecondNumParameters;
+    return true;
+  }
+
+  for (unsigned I = 0; I < FirstNumParameters; ++I) {
+    const ParmVarDecl *FirstParam = FirstMethod->getParamDecl(I);
+    const ParmVarDecl *SecondParam = SecondMethod->getParamDecl(I);
+
+    QualType FirstParamType = FirstParam->getType();
+    QualType SecondParamType = SecondParam->getType();
+    if (FirstParamType != SecondParamType &&
+        computeODRHash(FirstParamType) != computeODRHash(SecondParamType)) {
+      if (const DecayedType *ParamDecayedType =
+              FirstParamType->getAs<DecayedType>()) {
+        DiagError(ParameterType) << (I + 1) << FirstParamType << true
+                                 << ParamDecayedType->getOriginalType();
+      } else {
+        DiagError(ParameterType) << (I + 1) << FirstParamType << false;
+      }
+
+      if (const DecayedType *ParamDecayedType =
+              SecondParamType->getAs<DecayedType>()) {
+        DiagNote(ParameterType) << (I + 1) << SecondParamType << true
+                                << ParamDecayedType->getOriginalType();
+      } else {
+        DiagNote(ParameterType) << (I + 1) << SecondParamType << false;
+      }
+      return true;
+    }
+
+    DeclarationName FirstParamName = FirstParam->getDeclName();
+    DeclarationName SecondParamName = SecondParam->getDeclName();
+    if (FirstParamName != SecondParamName) {
+      DiagError(ParameterName) << (I + 1) << FirstParamName;
+      DiagNote(ParameterName) << (I + 1) << SecondParamName;
+      return true;
+    }
+  }
+
+  return false;
+}
+
 bool ODRDiagsEmitter::diagnoseSubMismatchField(
     const NamedDecl *FirstRecord, StringRef FirstModule, StringRef SecondModule,
     const FieldDecl *FirstField, const FieldDecl *SecondField) const {
   enum ODRFieldDifference {
     FieldName,
     FieldTypeName,
     FieldSingleBitField,
     FieldDifferentWidthBitField,
if (FirstProtocolName != SecondProtocolName) {
           << (I + 1) << SecondProtocolName;
       return true;
     }
   }
 
   return false;
 }
 
+bool ODRDiagsEmitter::diagnoseSubMismatchObjCMethod(
+    const NamedDecl *FirstObjCContainer, StringRef FirstModule,
+    StringRef SecondModule, const ObjCMethodDecl *FirstMethod,
+    const ObjCMethodDecl *SecondMethod) const {
+  enum ODRMethodDifference {
+    ReturnType,
+    InstanceOrClass,
+    ControlLevel, // optional/required
+    DesignatedInitializer,
+    Directness,
+    Name,
+  };
+
+  auto DiagError = [FirstObjCContainer, FirstModule, FirstMethod,
+                    this](ODRMethodDifference DiffType) {
+    return Diag(FirstMethod->getLocation(),
+                diag::err_module_odr_violation_objc_method)
+           << FirstObjCContainer << FirstModule.empty() << FirstModule
+           << FirstMethod->getSourceRange() << DiffType;
+  };
+  auto DiagNote = [SecondModule, SecondMethod,
+                   this](ODRMethodDifference DiffType) {
+    return Diag(SecondMethod->getLocation(),
+                diag::note_module_odr_violation_objc_method)
+           << SecondModule << SecondMethod->getSourceRange() << DiffType;
+  };
+
+  if (computeODRHash(FirstMethod->getReturnType()) !=
+      computeODRHash(SecondMethod->getReturnType())) {
+    DiagError(ReturnType) << FirstMethod << FirstMethod->getReturnType();
+    DiagNote(ReturnType) << SecondMethod << SecondMethod->getReturnType();
+    return true;
+  }
+
+  if (FirstMethod->isInstanceMethod() != SecondMethod->isInstanceMethod()) {
+    DiagError(InstanceOrClass)
+        << FirstMethod << FirstMethod->isInstanceMethod();
+    DiagNote(InstanceOrClass)
+        << SecondMethod << SecondMethod->isInstanceMethod();
+    return true;
+  }
+  if (FirstMethod->getImplementationControl() !=
+      SecondMethod->getImplementationControl()) {
+    DiagError(ControlLevel) << FirstMethod->getImplementationControl();
+    DiagNote(ControlLevel) << SecondMethod->getImplementationControl();
+    return true;
+  }
+  if (FirstMethod->isThisDeclarationADesignatedInitializer() !=
+      SecondMethod->isThisDeclarationADesignatedInitializer()) {
+    DiagError(DesignatedInitializer)
+        << FirstMethod
+        << FirstMethod->isThisDeclarationADesignatedInitializer();
+    DiagNote(DesignatedInitializer)
+        << SecondMethod
+        << SecondMethod->isThisDeclarationADesignatedInitializer();
+    return true;
+  }
+  if (FirstMethod->isDirectMethod() != SecondMethod->isDirectMethod()) {
+    DiagError(Directness) << FirstMethod << FirstMethod->isDirectMethod();
+    DiagNote(Directness) << SecondMethod << SecondMethod->isDirectMethod();
+    return true;
+  }
+  if (diagnoseSubMismatchMethodParameters(Diags, FirstObjCContainer,
+                                          FirstModule, SecondModule,
+                                          FirstMethod, SecondMethod))
+    return true;
+
+  // Check method name *after* looking at the parameters otherwise we get a
+  // less ideal diagnostics: a ObjCMethodName mismatch given that selectors
+  // for different parameters are likely to be different.
+  DeclarationName FirstName = FirstMethod->getDeclName();
+  DeclarationName SecondName = SecondMethod->getDeclName();
+  if (FirstName != SecondName) {
+    DiagError(Name) << FirstName;
+    DiagNote(Name) << SecondName;
+    return true;
+  }
+
+  return false;
+}
+
 ODRDiagsEmitter::DiffResult
 ODRDiagsEmitter::FindTypeDiffs(DeclHashes &FirstHashes,
                                DeclHashes &SecondHashes) {
   auto DifferenceSelector = [](const Decl *D) {
     assert(D && "valid Decl required");
     switch (D->getKind()) {
     default:
       return Other;
auto DifferenceSelector = [](const Decl *D) {
     case Decl::Typedef:
       return TypeDef;
     case Decl::Var:
       return Var;
     case Decl::Friend:
       return Friend;
     case Decl::FunctionTemplate:
       return FunctionTemplate;
+    case Decl::ObjCMethod:
+      return ObjCMethod;
     }
   };
 
   DiffResult DR;
   auto FirstIt = FirstHashes.begin();
   auto SecondIt = SecondHashes.begin();
   while (FirstIt != FirstHashes.end() || SecondIt != SecondHashes.end()) {
     if (FirstIt != FirstHashes.end() && SecondIt != SecondHashes.end() &&

 
 void ODRDiagsEmitter::diagnoseSubMismatchDifferentDeclKinds(
     DiffResult &DR, const NamedDecl *FirstRecord, StringRef FirstModule,
     const NamedDecl *SecondRecord, StringRef SecondModule) const {
   auto GetMismatchedDeclLoc = [](const NamedDecl *Container,
                                  ODRMismatchDecl DiffType, const Decl *D) {
     SourceLocation Loc;
     SourceRange Range;
-    auto *Tag = dyn_cast<TagDecl>(Container);
-    if (DiffType == EndOfClass && Tag) {
+    if (DiffType == EndOfClass) {
+      if (auto *Tag = dyn_cast<TagDecl>(Container))
         Loc = Tag->getBraceRange().getEnd();
+      else
+        Loc = Container->getEndLoc();
     } else {
       Loc = D->getLocation();
       Range = D->getSourceRange();
     }
     return std::make_pair(Loc, Range);
   };
 
   auto FirstDiagInfo =
enum ODRCXXRecordDifference {
     MethodName,
     MethodDeleted,
     MethodDefaulted,
     MethodVirtual,
     MethodStatic,
     MethodVolatile,
     MethodConst,
     MethodInline,
-    MethodNumberParameters,
-    MethodParameterType,
-    MethodParameterName,
     MethodParameterSingleDefaultArgument,
     MethodParameterDifferentDefaultArgument,
     MethodNoTemplateArguments,
     MethodDifferentNumberTemplateArguments,
     MethodDifferentTemplateArgument,
     MethodSingleBody,
     MethodDifferentBody,
     FriendTypeFunction,
bool ODRDiagsEmitter::diagnoseMismatch(
 
   assert(FirstDiffType == SecondDiffType);
   switch (FirstDiffType) {
   case Other:
   case EndOfClass:
   case PublicSpecifer:
   case PrivateSpecifer:
   case ProtectedSpecifer:
+  case ObjCMethod:
     llvm_unreachable("Invalid diff type");
 
   case StaticAssert: {
     const StaticAssertDecl *FirstSA = cast<StaticAssertDecl>(FirstDecl);
     const StaticAssertDecl *SecondSA = cast<StaticAssertDecl>(SecondDecl);
 
     const Expr *FirstExpr = FirstSA->getAssertExpr();
     const Expr *SecondExpr = SecondSA->getAssertExpr();
case CXXMethod: {
     const bool FirstInline = FirstMethod->isInlineSpecified();
     const bool SecondInline = SecondMethod->isInlineSpecified();
     if (FirstInline != SecondInline) {
       DiagMethodError(MethodInline) << FirstInline;
       DiagMethodNote(MethodInline) << SecondInline;
       return true;
     }
 
-    const unsigned FirstNumParameters = FirstMethod->param_size();
-    const unsigned SecondNumParameters = SecondMethod->param_size();
-    if (FirstNumParameters != SecondNumParameters) {
-      DiagMethodError(MethodNumberParameters) << FirstNumParameters;
-      DiagMethodNote(MethodNumberParameters) << SecondNumParameters;
+    if (diagnoseSubMismatchMethodParameters(Diags, FirstRecord,
+                                            FirstModule, SecondModule,
+                                            FirstMethod, SecondMethod))
       return true;
-    }
 
-    for (unsigned I = 0; I < FirstNumParameters; ++I) {
+    for (unsigned I = 0, N = FirstMethod->param_size(); I < N; ++I) {
       const ParmVarDecl *FirstParam = FirstMethod->getParamDecl(I);
       const ParmVarDecl *SecondParam = SecondMethod->getParamDecl(I);
 
-      QualType FirstParamType = FirstParam->getType();
-      QualType SecondParamType = SecondParam->getType();
-      if (FirstParamType != SecondParamType &&
-          computeODRHash(FirstParamType) != computeODRHash(SecondParamType)) {
-        if (const DecayedType *ParamDecayedType =
-                FirstParamType->getAs<DecayedType>()) {
-          DiagMethodError(MethodParameterType)
-              << (I + 1) << FirstParamType << true
-              << ParamDecayedType->getOriginalType();
-        } else {
-          DiagMethodError(MethodParameterType)
-              << (I + 1) << FirstParamType << false;
-        }
-
-        if (const DecayedType *ParamDecayedType =
-                SecondParamType->getAs<DecayedType>()) {
-          DiagMethodNote(MethodParameterType)
-              << (I + 1) << SecondParamType << true
-              << ParamDecayedType->getOriginalType();
-        } else {
-          DiagMethodNote(MethodParameterType)
-              << (I + 1) << SecondParamType << false;
-        }
-        return true;
-      }
-
-      DeclarationName FirstParamName = FirstParam->getDeclName();
-      DeclarationName SecondParamName = SecondParam->getDeclName();
-      if (FirstParamName != SecondParamName) {
-        DiagMethodError(MethodParameterName) << (I + 1) << FirstParamName;
-        DiagMethodNote(MethodParameterName) << (I + 1) << SecondParamName;
-        return true;
-      }
-
       const Expr *FirstInit = FirstParam->getInit();
       const Expr *SecondInit = SecondParam->getInit();
       if ((FirstInit == nullptr) != (SecondInit == nullptr)) {
         DiagMethodError(MethodParameterSingleDefaultArgument)
             << (I + 1) << (FirstInit == nullptr)
             << (FirstInit ? FirstInit->getSourceRange() : SourceRange());
         DiagMethodNote(MethodParameterSingleDefaultArgument)
             << (I + 1) << (SecondInit == nullptr)
bool ODRDiagsEmitter::diagnoseMismatch(
   case PrivateSpecifer:
   case ProtectedSpecifer:
   case StaticAssert:
   case CXXMethod:
   case TypeAlias:
   case Friend:
   case FunctionTemplate:
     llvm_unreachable("Invalid diff type");
+  case ObjCMethod: {
+    if (diagnoseSubMismatchObjCMethod(FirstProtocol, FirstModule, SecondModule,
+                                      cast<ObjCMethodDecl>(FirstDecl),
+                                      cast<ObjCMethodDecl>(SecondDecl)))
+      return true;
+    break;
+  }
   }
 
   Diag(FirstDecl->getLocation(),
        diag::err_module_odr_violation_mismatch_decl_unknown)
       << FirstProtocol << FirstModule.empty() << FirstModule << FirstDiffType
       << FirstDecl->getSourceRange();
   Diag(SecondDecl->getLocation(),
        diag::note_module_odr_violation_mismatch_decl_unknown)
       << SecondModule << FirstDiffType << SecondDecl->getSourceRange();
   return true;
 }

clang/lib/AST/ODRHash.cpp

void ODRHash::AddDeclarationNameImpl(DeclarationName Name) {
   case DeclarationName::ObjCOneArgSelector:
   case DeclarationName::ObjCMultiArgSelector: {
     Selector S = Name.getObjCSelector();
     AddBoolean(S.isNull());
     AddBoolean(S.isKeywordSelector());
     AddBoolean(S.isUnarySelector());
     unsigned NumArgs = S.getNumArgs();
     ID.AddInteger(NumArgs);
-    for (unsigned i = 0; i < NumArgs; ++i) {
+    // Compare all selector slots. For selectors with arguments it means all arg
+    // slots. And if there are no arguments, compare the first-and-only slot.
+    unsigned SlotsToCheck = NumArgs > 0 ? NumArgs : 1;
+    for (unsigned i = 0; i < SlotsToCheck; ++i) {
       const IdentifierInfo *II = S.getIdentifierInfoForSlot(i);
       AddBoolean(II);
       if (II) {
         AddIdentifierInfo(II);
       }
     }
     break;
   }
public:
   }
 
   void VisitCXXMethodDecl(const CXXMethodDecl *D) {
     // Handled by the ODRHash for FunctionDecl
 
     Inherited::VisitCXXMethodDecl(D);
   }
 
+  void VisitObjCMethodDecl(const ObjCMethodDecl *Method) {
+    ID.AddInteger(Method->getDeclKind());
+    Hash.AddBoolean(Method->isInstanceMethod()); // false if class method
+    Hash.AddBoolean(Method->isPropertyAccessor());
+    Hash.AddBoolean(Method->isVariadic());
+    Hash.AddBoolean(Method->isSynthesizedAccessorStub());
+    Hash.AddBoolean(Method->isDefined());
+    Hash.AddBoolean(Method->isOverriding());
+    Hash.AddBoolean(Method->isDirectMethod());
+    Hash.AddBoolean(Method->isThisDeclarationADesignatedInitializer());
+    Hash.AddBoolean(Method->hasSkippedBody());
+
+    ID.AddInteger(Method->getImplementationControl());
+    ID.AddInteger(Method->getMethodFamily());
+    ImplicitParamDecl *Cmd = Method->getCmdDecl();
+    Hash.AddBoolean(Cmd);
+    if (Cmd)
+      ID.AddInteger(Cmd->getParameterKind());
+
+    ImplicitParamDecl *Self = Method->getSelfDecl();
+    Hash.AddBoolean(Self);
+    if (Self)
+      ID.AddInteger(Self->getParameterKind());
+
+    AddDecl(Method);
+
+    AddQualType(Method->getReturnType());
+    ID.AddInteger(Method->param_size());
+    for (auto Param : Method->parameters())
+      Hash.AddSubDecl(Param);
+
+    if (Method->hasBody()) {
+      const bool IsDefinition = Method->isThisDeclarationADefinition();
+      Hash.AddBoolean(IsDefinition);
+      if (IsDefinition) {
+        Stmt *Body = Method->getBody();
+        Hash.AddBoolean(Body);
+        if (Body)
+          AddStmt(Body);
+
+        // Filter out sub-Decls which will not be processed in order to get an
+        // accurate count of Decl's.
+        llvm::SmallVector<const Decl *, 16> Decls;
+        for (Decl *SubDecl : Method->decls())
+          if (ODRHash::isSubDeclToBeProcessed(SubDecl, Method))
+            Decls.push_back(SubDecl);
+
+        ID.AddInteger(Decls.size());
+        for (auto SubDecl : Decls)
+          Hash.AddSubDecl(SubDecl);
+      }
+    } else {
+      Hash.AddBoolean(false);
+    }
+
+    Inherited::VisitObjCMethodDecl(Method);
+  }
+
   void VisitTypedefNameDecl(const TypedefNameDecl *D) {
     AddQualType(D->getUnderlyingType());
 
     Inherited::VisitTypedefNameDecl(D);
   }
 
   void VisitTypedefDecl(const TypedefDecl *D) {
     Inherited::VisitTypedefDecl(D);
switch (D->getKind()) {
     case Decl::EnumConstant: // Only found in EnumDecl's.
     case Decl::Field:
     case Decl::Friend:
     case Decl::FunctionTemplate:
     case Decl::StaticAssert:
     case Decl::TypeAlias:
     case Decl::Typedef:
     case Decl::Var:
+    case Decl::ObjCMethod:
       return true;
   }
 }
 
 void ODRHash::AddSubDecl(const Decl *D) {
   assert(D && "Expecting non-null pointer.");
 
   ODRDeclVisitor(ID, *this).Visit(D);

clang/test/Modules/compare-objc-protocol.m

 
 id<CompareDifferentProtocols> compareDifferentProtocols;
 // expected-error@first.h:* {{'CompareDifferentProtocols' has different definitions in different modules; first difference is definition in module 'First.Hidden' found 1st referenced protocol with name 'CommonProtocol'}}
 // expected-note@second.h:* {{but in 'Second' found 1st referenced protocol with different name 'ExtraProtocol'}}
 id<CompareProtocolOrder> compareProtocolOrder;
 // expected-error@first.h:* {{'CompareProtocolOrder' has different definitions in different modules; first difference is definition in module 'First.Hidden' found 1st referenced protocol with name 'CommonProtocol'}}
 // expected-note@second.h:* {{but in 'Second' found 1st referenced protocol with different name 'ExtraProtocol'}}
 #endif
+
+#if defined(FIRST)
+@protocol CompareMatchingMethods
+- (float)matchingMethod:(int)arg;
+@end
+
+@protocol CompareMethodPresence1
+- (void)presenceMethod1;
+@end
+@protocol CompareMethodPresence2
+@end
+
+@protocol CompareMethodName
+- (void)methodNameA;
+@end
+
+@protocol CompareMethodArgCount
+- (void)methodArgCount:(int)arg0 :(int)arg1;
+@end
+@protocol CompareMethodArgName
+- (void)methodArgName:(int)argNameA;
+@end
+@protocol CompareMethodArgType
+- (void)methodArgType:(int)argType;
+@end
+
+@protocol CompareMethodReturnType
+- (int)methodReturnType;
+@end
+
+@protocol CompareMethodOrder
+- (void)methodOrderFirst;
+- (void)methodOrderSecond;
+@end
+
+@protocol CompareMethodClassInstance
+- (void)methodClassInstance;
+@end
+
+@protocol CompareMethodRequirednessExplicit
+@optional
+- (void)methodRequiredness;
+@end
+@protocol CompareMethodRequirednessDefault
+// @required is default
+- (void)methodRequiredness;
+@end
+#elif defined(SECOND)
+@protocol CompareMatchingMethods
+- (float)matchingMethod:(int)arg;
+@end
+
+@protocol CompareMethodPresence1
+@end
+@protocol CompareMethodPresence2
+- (void)presenceMethod2;
+@end
+
+@protocol CompareMethodName
+- (void)methodNameB;
+@end
+
+@protocol CompareMethodArgCount
+- (void)methodArgCount:(int)arg0;
+@end
+@protocol CompareMethodArgName
+- (void)methodArgName:(int)argNameB;
+@end
+@protocol CompareMethodArgType
+- (void)methodArgType:(float)argType;
+@end
+
+@protocol CompareMethodReturnType
+- (float)methodReturnType;
+@end
+
+@protocol CompareMethodOrder
+- (void)methodOrderSecond;
+- (void)methodOrderFirst;
+@end
+
+@protocol CompareMethodClassInstance
++ (void)methodClassInstance;
+@end
+
+@protocol CompareMethodRequirednessExplicit
+@required
+- (void)methodRequiredness;
+@end
+@protocol CompareMethodRequirednessDefault
+@required
+- (void)methodRequiredness;
+@end
+#else
+id<CompareMatchingMethods> compareMatchingMethods; // no error
+id<CompareMethodPresence1> compareMethodPresence1;
+// expected-error@first.h:* {{'CompareMethodPresence1' has different definitions in different modules; first difference is definition in module 'First.Hidden' found method}}
+// expected-note@second.h:* {{but in 'Second' found end of class}}
+id<CompareMethodPresence2> compareMethodPresence2;
+// expected-error@first.h:* {{'CompareMethodPresence2' has different definitions in different modules; first difference is definition in module 'First.Hidden' found end of class}}
+// expected-note@second.h:* {{but in 'Second' found method}}
+id<CompareMethodName> compareMethodName;
+// expected-error@first.h:* {{'CompareMethodName' has different definitions in different modules; first difference is definition in module 'First.Hidden' found method 'methodNameA'}}
+// expected-note@second.h:* {{but in 'Second' found different method 'methodNameB'}}
+
+id<CompareMethodArgCount> compareMethodArgCount;
+// expected-error@first.h:* {{'CompareMethodArgCount' has different definitions in different modules; first difference is definition in module 'First.Hidden' found method 'methodArgCount::' that has 2 parameters}}
+// expected-note@second.h:* {{but in 'Second' found method 'methodArgCount:' that has 1 parameter}}
+id<CompareMethodArgName> compareMethodArgName;
+// expected-error@first.h:* {{'CompareMethodArgName' has different definitions in different modules; first difference is definition in module 'First.Hidden' found method 'methodArgName:' with 1st parameter named 'argNameA'}}
+// expected-note@second.h:* {{but in 'Second' found method 'methodArgName:' with 1st parameter named 'argNameB'}}
+id<CompareMethodArgType> compareMethodArgType;
+// expected-error@first.h:* {{'CompareMethodArgType' has different definitions in different modules; first difference is definition in module 'First.Hidden' found method 'methodArgType:' with 1st parameter of type 'int'}}
+// expected-note@second.h:* {{but in 'Second' found method 'methodArgType:' with 1st parameter of type 'float'}}
+
+id<CompareMethodReturnType> compareMethodReturnType;
+// expected-error@first.h:* {{'CompareMethodReturnType' has different definitions in different modules; first difference is definition in module 'First.Hidden' found method 'methodReturnType' with return type 'int'}}
+// expected-note@second.h:* {{but in 'Second' found method 'methodReturnType' with different return type 'float'}}
+
+id<CompareMethodOrder> compareMethodOrder;
+// expected-error@first.h:* {{'CompareMethodOrder' has different definitions in different modules; first difference is definition in module 'First.Hidden' found method 'methodOrderFirst'}}
+// expected-note@second.h:* {{but in 'Second' found different method 'methodOrderSecond'}}
+id<CompareMethodClassInstance> compareMethodClassInstance;
+// expected-error@first.h:* {{'CompareMethodClassInstance' has different definitions in different modules; first difference is definition in module 'First.Hidden' found instance method 'methodClassInstance'}}
+// expected-note@second.h:* {{but in 'Second' found method 'methodClassInstance' as class method}}
+
+id<CompareMethodRequirednessExplicit> compareMethodRequirednessExplicit;
+// expected-error@first.h:* {{'CompareMethodRequirednessExplicit' has different definitions in different modules; first difference is definition in module 'First.Hidden' found 'optional' method control}}
+// expected-note@second.h:* {{but in 'Second' found 'required' method control}}
+id<CompareMethodRequirednessDefault> compareMethodRequirednessDefault; // no error
+#endif