[ODRHash diagnostics] Transform method `ASTReader::diagnoseOdrViolations` into a class `ODRDiagsEmitter`. NFC.

[vsapsai]

Summary

Preparing to use diagnostics about ODR hash mismatches outside of ASTReader.

[vsapsai]

Rebase on a parent change.

[vsapsai]

Rebase more.

[vsapsai]

Last rebase on top of prep change.

[vsapsai]

Rebase.

[vsapsai]

Sorry for the huge amount of changes in ASTReader.cpp but most of them are indentation changes, so I hope it's not too bad. I've tried to minimize the changes unrelated to introducing ODRDiagsEmitter but if anybody has other ideas/requests, I'm totally willing to reduce the load on reviewers.

[ChuanqiXu]

In D128490#3670441, @vsapsai wrote:

Sorry for the huge amount of changes in ASTReader.cpp but most of them are indentation changes, so I hope it's not too bad. I've tried to minimize the changes unrelated to introducing ODRDiagsEmitter but if anybody has other ideas/requests, I'm totally willing to reduce the load on reviewers.

For these indentation changes, maybe it is a good idea to edit the .git-blame-ignore-revs files in the root path.

[vsapsai]

Add doc comments to public methods in ODRDiagsEmitter.

[vsapsai]

In D128490#3670543, @ChuanqiXu wrote:

For these indentation changes, maybe it is a good idea to edit the .git-blame-ignore-revs files in the root path.

That is a good idea. I'm thinking about a different option: I make my change but keep ODRDiagsEmitter methods over-indented to preserve the old indentation. The code is moved to a different file in D128695 and I just format the entire file. This way the current change is easier to review (now and in git history) and the formatting is [eventually] correct. As for me, having good git blame is more important than having perfect formatting (proposed formatting will be still OK).

[vsapsai]

• Revert to the old indentation.

[vsapsai]

• Who could have suspected that some lambdas were double-indented.

[vsapsai]

• Replicate some creative old indentation.

[vsapsai]

• Revert indentation for the second and subsequent lines in lambda-now-method calls.

[vsapsai]

OK. Now I've reverted mostly to the old indentation, preserving some of the quirks. Hope it is easier to review now.

[ChuanqiXu]

clang/lib/Serialization/ASTReader.cpp

9569–9576

We add a static method with the same name of the original one but we don't remove the original one. It looks odd. Could we remove the odd one?

9728–9729

Could we hoist these construction?

[vsapsai]

clang/lib/Serialization/ASTReader.cpp

9569–9576

Hmm, I've tried removing the old one ASTReader::getOwningModuleNameForDiagnostic but some tests were failing. Now all the tests seem to be fine. I'll remove ASTReader::getOwningModuleNameForDiagnostic and we'll see if pre-commit tests are happy.

9728–9729

Sure. I see your point that ODRDiagsEmitter is stateless regarding diagnoseMismatch methods, so repeating the same construction looks excessive and verbose. But if anybody later has any reasons to change that, it should be possible as ODRDiagsEmitter construction is cheap.

[vsapsai]

• Address review comments.

[vsapsai]

clang/lib/Serialization/ASTReader.cpp

9194–9196

At first I though this part was important for certain tests. But I guess I've messed up somewhere else.

[ChuanqiXu]

LGTM then.

[vsapsai]

In D128490#3689958, @ChuanqiXu wrote:

LGTM then.

Thanks for the review! Sorry for not answering earlier, I was on vacation. Now I'll rebase the change and will test it again before committing.

[vsapsai]

Rebase

clang/include/clang/AST/DeclCXX.h

class CXXRecordDecl : public RecordDecl {
   friend class ASTDeclReader;
   friend class ASTDeclWriter;
   friend class ASTNodeImporter;
   friend class ASTReader;
   friend class ASTRecordWriter;
   friend class ASTWriter;
   friend class DeclContext;
   friend class LambdaExpr;
+  friend class ODRDiagsEmitter;
 
   friend void FunctionDecl::setPure(bool);
   friend void TagDecl::startDefinition();
 
   /// Values used in DefinitionData fields to represent special members.
   enum SpecialMemberFlags {
     SMF_DefaultConstructor = 0x1,
     SMF_CopyConstructor = 0x2,

clang/include/clang/Serialization/ASTReader.h

public:
 
   /// Returns true if global DeclID \p ID originated from module \p M.
   bool isDeclIDFromModule(serialization::GlobalDeclID ID, ModuleFile &M) const;
 
   /// Retrieve the module file that owns the given declaration, or NULL
   /// if the declaration is not from a module file.
   ModuleFile *getOwningModuleFile(const Decl *D);
 
-  /// 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.
-  std::string getOwningModuleNameForDiagnostic(const Decl *D);
-
   /// Returns the source location for the decl \p ID.
   SourceLocation getSourceLocationForDeclID(serialization::GlobalDeclID ID);
 
   /// Resolve a declaration ID into a declaration, potentially
   /// building a new declaration.
   Decl *GetDecl(serialization::DeclID ID);
   Decl *GetExternalDecl(uint32_t ID) override;
 

clang/lib/Serialization/ASTReader.cpp

for (unsigned I = 0; I < NumInputs; ++I) {
     InputFileInfo IFI = readInputFileInfo(MF, I + 1);
     if (IFI.TopLevelModuleMap)
       // FIXME: This unnecessarily re-reads the InputFileInfo.
       if (auto FE = getInputFile(MF, I + 1).getFile())
         Visitor(FE);
   }
 }
 
-std::string ASTReader::getOwningModuleNameForDiagnostic(const Decl *D) {
-  // If we know the owning module, use it.
-  if (Module *M = D->getImportedOwningModule())
-    return M->getFullModuleName();
-
-  // Otherwise, use the name of the top-level module the decl is within.
-  if (ModuleFile *M = getOwningModuleFile(D))
-    return M->ModuleName;
-
-  // Not from a module.
-  return {};
-}
-
 void ASTReader::finishPendingActions() {
   while (!PendingIdentifierInfos.empty() || !PendingFunctionTypes.empty() ||
          !PendingIncompleteDeclChains.empty() || !PendingDeclChains.empty() ||
          !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
          !PendingUpdateRecords.empty() ||
          !PendingObjCExtensionIvarRedeclarations.empty()) {
     // If any identifiers with corresponding top-level declarations have
     // been loaded, load those declarations now.
void ASTReader::finishPendingActions() {
   PendingBodies.clear();
 
   // Do some cleanup.
   for (auto *ND : PendingMergedDefinitionsToDeduplicate)
     getContext().deduplicateMergedDefinitonsFor(ND);
   PendingMergedDefinitionsToDeduplicate.clear();
 }
 
+namespace clang {
+class ODRDiagsEmitter {
+public:
+  ODRDiagsEmitter(DiagnosticsEngine &Diags, const ASTContext &Context,
+                  const LangOptions &LangOpts)
+      : Diags(Diags), Context(Context), LangOpts(LangOpts) {}
+
+  /// Diagnose ODR mismatch between 2 FunctionDecl.
+  ///
+  /// Returns true if found a mismatch and diagnosed it.
+  bool diagnoseMismatch(const FunctionDecl *FirstFunction,
+                        const FunctionDecl *SecondFunction) const;
+
+  /// Diagnose ODR mismatch between 2 EnumDecl.
+  ///
+  /// Returns true if found a mismatch and diagnosed it.
+  bool diagnoseMismatch(const EnumDecl *FirstEnum,
+                        const EnumDecl *SecondEnum) const;
+
+  /// Diagnose ODR mismatch between 2 CXXRecordDecl.
+  ///
+  /// Returns true if found a mismatch and diagnosed it.
+  /// To compare 2 declarations with merged and identical definition data
+  /// you need to provide pre-merge definition data in \p SecondDD.
+  bool
+  diagnoseMismatch(const CXXRecordDecl *FirstRecord,
+                   const CXXRecordDecl *SecondRecord,
+                   const struct CXXRecordDecl::DefinitionData *SecondDD) const;
+
+  /// 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
+  // This list should be the same Decl's as in ODRHash::isDeclToBeProcessed
+  enum ODRMismatchDecl {
+    EndOfClass,
+    PublicSpecifer,
+    PrivateSpecifer,
+    ProtectedSpecifer,
+    StaticAssert,
+    Field,
+    CXXMethod,
+    TypeAlias,
+    TypeDef,
+    Var,
+    Friend,
+    FunctionTemplate,
+    Other
+  };
+
+  struct DiffResult {
+    const Decl *FirstDecl = nullptr, *SecondDecl = nullptr;
+    ODRMismatchDecl FirstDiffType = Other, SecondDiffType = Other;
+  };
+
+  // If there is a diagnoseable difference, FirstDiffType and
+  // SecondDiffType will not be Other and FirstDecl and SecondDecl will be
+  // filled in if not EndOfClass.
+  static DiffResult FindTypeDiffs(DeclHashes &FirstHashes,
+                                  DeclHashes &SecondHashes);
+
+  DiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID) const {
+    return Diags.Report(Loc, DiagID);
+  }
+
+  // Use this to diagnose that an unexpected Decl was encountered
+  // or no difference was detected. This causes a generic error
+  // message to be emitted.
+  void diagnoseSubMismatchUnexpected(DiffResult &DR,
+                                     const NamedDecl *FirstRecord,
+                                     StringRef FirstModule,
+                                     const NamedDecl *SecondRecord,
+                                     StringRef SecondModule) const;
+
+  void diagnoseSubMismatchDifferentDeclKinds(DiffResult &DR,
+                                             const NamedDecl *FirstRecord,
+                                             StringRef FirstModule,
+                                             const NamedDecl *SecondRecord,
+                                             StringRef SecondModule) const;
+
+  bool diagnoseSubMismatchField(const NamedDecl *FirstRecord,
+                                StringRef FirstModule, StringRef SecondModule,
+                                const FieldDecl *FirstField,
+                                const FieldDecl *SecondField) const;
+
+  bool diagnoseSubMismatchTypedef(const NamedDecl *FirstRecord,
+                                  StringRef FirstModule, StringRef SecondModule,
+                                  const TypedefNameDecl *FirstTD,
+                                  const TypedefNameDecl *SecondTD,
+                                  bool IsTypeAlias) const;
+
+  bool diagnoseSubMismatchVar(const NamedDecl *FirstRecord,
+                              StringRef FirstModule, StringRef SecondModule,
+                              const VarDecl *FirstVD,
+                              const VarDecl *SecondVD) const;
+
+private:
+  DiagnosticsEngine &Diags;
+  const ASTContext &Context;
+  const LangOptions &LangOpts;
+};
+} // namespace clang
+
 static unsigned computeODRHash(QualType Ty) {
   ODRHash Hasher;
   Hasher.AddQualType(Ty);
   return Hasher.CalculateHash();
 }
 
 static unsigned computeODRHash(const Stmt *S) {
   ODRHash Hasher;

 }
 
 static unsigned computeODRHash(const TemplateArgument &TA) {
   ODRHash Hasher;
   Hasher.AddTemplateArgument(TA);
   return Hasher.CalculateHash();
 }
 
+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 {};
+}
+
 void ASTReader::diagnoseOdrViolations() {
   if (PendingOdrMergeFailures.empty() && PendingOdrMergeChecks.empty() &&
       PendingFunctionOdrMergeFailures.empty() &&
       PendingEnumOdrMergeFailures.empty())
     return;
 
   // Trigger the import of the full definition of each class that had any
   // odr-merging problems, so we can produce better diagnostics for them.
if (!Found) {
       if (!isa<TagDecl>(D))
         D->setInvalidDecl();
 
       // Ensure we don't accidentally recursively enter deserialization while
       // we're producing our diagnostic.
       Deserializing RecursionGuard(this);
 
       std::string CanonDefModule =
-          getOwningModuleNameForDiagnostic(cast<Decl>(CanonDef));
+          ODRDiagsEmitter::getOwningModuleNameForDiagnostic(
+              cast<Decl>(CanonDef));
       Diag(D->getLocation(), diag::err_module_odr_violation_missing_decl)
-        << D << getOwningModuleNameForDiagnostic(D)
+        << D << ODRDiagsEmitter::getOwningModuleNameForDiagnostic(D)
         << CanonDef << CanonDefModule.empty() << CanonDefModule;
 
       if (Candidates.empty())
         Diag(cast<Decl>(CanonDef)->getLocation(),
              diag::note_module_odr_violation_no_possible_decls) << D;
       else {
         for (unsigned I = 0, N = Candidates.size(); I != N; ++I)
           Diag(Candidates[I]->getLocation(),
                diag::note_module_odr_violation_possible_decl)
             << Candidates[I];
       }
 
       DiagnosedOdrMergeFailures.insert(CanonDef);
     }
   }
 
   if (OdrMergeFailures.empty() && FunctionOdrMergeFailures.empty() &&
       EnumOdrMergeFailures.empty())
     return;
 
   // Ensure we don't accidentally recursively enter deserialization while
   // we're producing our diagnostics.
   Deserializing RecursionGuard(this);
+  ODRDiagsEmitter DiagsEmitter(Diags, getContext(),
+                               getPreprocessor().getLangOpts());
 
-  // Used with err_module_odr_violation_mismatch_decl and
-  // note_module_odr_violation_mismatch_decl
-  // This list should be the same Decl's as in ODRHash::isDeclToBeProcessed
-  enum ODRMismatchDecl {
-    EndOfClass,
-    PublicSpecifer,
-    PrivateSpecifer,
-    ProtectedSpecifer,
-    StaticAssert,
-    Field,
-    CXXMethod,
-    TypeAlias,
-    TypeDef,
-    Var,
-    Friend,
-    FunctionTemplate,
-    Other
-  };
+  // Issue any pending ODR-failure diagnostics.
+  for (auto &Merge : OdrMergeFailures) {
+    // If we've already pointed out a specific problem with this class, don't
+    // bother issuing a general "something's different" diagnostic.
+    if (!DiagnosedOdrMergeFailures.insert(Merge.first).second)
+      continue;
 
-  // These lambdas have the common portions of the ODR diagnostics.  This
-  // has the same return as Diag(), so addition parameters can be passed
-  // in with operator<<
-  auto ODRDiagField = [this](NamedDecl *FirstRecord, StringRef FirstModule,
-                             StringRef SecondModule,
-                             const FieldDecl *FirstField,
-                             const FieldDecl *SecondField) {
+    bool Diagnosed = false;
+    CXXRecordDecl *FirstRecord = Merge.first;
+    for (auto &RecordPair : Merge.second) {
+      if (DiagsEmitter.diagnoseMismatch(FirstRecord, RecordPair.first,
+                                        RecordPair.second)) {
+        Diagnosed = true;
+        break;
+      }
+    }
+
+    if (!Diagnosed) {
+      // All definitions are updates to the same declaration. This happens if a
+      // module instantiates the declaration of a class template specialization
+      // and two or more other modules instantiate its definition.
+      //
+      // FIXME: Indicate which modules had instantiations of this definition.
+      // FIXME: How can this even happen?
+      Diag(Merge.first->getLocation(),
+           diag::err_module_odr_violation_different_instantiations)
+          << Merge.first;
+    }
+  }
+
+  // Issue ODR failures diagnostics for functions.
+  for (auto &Merge : FunctionOdrMergeFailures) {
+    FunctionDecl *FirstFunction = Merge.first;
+    bool Diagnosed = false;
+    for (auto &SecondFunction : Merge.second) {
+      if (DiagsEmitter.diagnoseMismatch(FirstFunction, SecondFunction)) {
+        Diagnosed = true;
+        break;
+      }
+    }
+    (void)Diagnosed;
+    assert(Diagnosed && "Unable to emit ODR diagnostic.");
+  }
+
+  // Issue ODR failures diagnostics for enums.
+  for (auto &Merge : EnumOdrMergeFailures) {
+    // If we've already pointed out a specific problem with this enum, don't
+    // bother issuing a general "something's different" diagnostic.
+    if (!DiagnosedOdrMergeFailures.insert(Merge.first).second)
+      continue;
+
+    EnumDecl *FirstEnum = Merge.first;
+    bool Diagnosed = false;
+    for (auto &SecondEnum : Merge.second) {
+      if (DiagsEmitter.diagnoseMismatch(FirstEnum, SecondEnum)) {
+        Diagnosed = true;
+        break;
+      }
+    }
+    (void)Diagnosed;
+    assert(Diagnosed && "Unable to emit ODR diagnostic.");
+  }
+}
+
+// clang-format off
+  bool ODRDiagsEmitter::diagnoseSubMismatchField(const NamedDecl *FirstRecord,
+      StringRef FirstModule, StringRef SecondModule,
+      const FieldDecl *FirstField, const FieldDecl *SecondField) const {
     enum ODRFieldDifference {
       FieldName,
       FieldTypeName,
       FieldSingleBitField,
       FieldDifferentWidthBitField,
       FieldSingleMutable,
       FieldSingleInitializer,
       FieldDifferentInitializers,
bool ODRDiagsEmitter::diagnoseSubMismatchField(const NamedDecl *FirstRecord,
     IdentifierInfo *FirstII = FirstField->getIdentifier();
     IdentifierInfo *SecondII = SecondField->getIdentifier();
     if (FirstII->getName() != SecondII->getName()) {
       DiagError(FieldName) << FirstII;
       DiagNote(FieldName) << SecondII;
       return true;
     }
 
-    assert(getContext().hasSameType(FirstField->getType(),
-                                    SecondField->getType()));
+    assert(Context.hasSameType(FirstField->getType(), SecondField->getType()));
 
     QualType FirstType = FirstField->getType();
     QualType SecondType = SecondField->getType();
     if (computeODRHash(FirstType) != computeODRHash(SecondType)) {
       DiagError(FieldTypeName) << FirstII << FirstType;
       DiagNote(FieldTypeName) << SecondII << SecondType;
       return true;
     }
if (IsFirstBitField && IsSecondBitField) {
         DiagError(FieldDifferentWidthBitField)
             << FirstII << FirstField->getBitWidth()->getSourceRange();
         DiagNote(FieldDifferentWidthBitField)
             << SecondII << SecondField->getBitWidth()->getSourceRange();
         return true;
       }
     }
 
-    if (!PP.getLangOpts().CPlusPlus)
+    if (!LangOpts.CPlusPlus)
       return false;
 
     const bool IsFirstMutable = FirstField->isMutable();
     const bool IsSecondMutable = SecondField->isMutable();
     if (IsFirstMutable != IsSecondMutable) {
       DiagError(FieldSingleMutable) << FirstII << IsFirstMutable;
       DiagNote(FieldSingleMutable) << SecondII << IsSecondMutable;
       return true;
if (FirstInitializer && SecondInitializer) {
             << FirstII << FirstInitializer->getSourceRange();
         DiagNote(FieldDifferentInitializers)
             << SecondII << SecondInitializer->getSourceRange();
         return true;
       }
     }
 
     return false;
-  };
+  }
 
-  auto ODRDiagTypeDefOrAlias =
-      [this](NamedDecl *FirstRecord, StringRef FirstModule,
-             StringRef SecondModule, const TypedefNameDecl *FirstTD,
-             const TypedefNameDecl *SecondTD, bool IsTypeAlias) {
+  bool ODRDiagsEmitter::diagnoseSubMismatchTypedef(const NamedDecl *FirstRecord,
+      StringRef FirstModule, StringRef SecondModule,
+      const TypedefNameDecl *FirstTD, const TypedefNameDecl *SecondTD,
+      bool IsTypeAlias) const {
         enum ODRTypedefDifference {
           TypedefName,
           TypedefType,
         };
 
         auto DiagError = [FirstRecord, FirstTD, FirstModule,
                           this](ODRTypedefDifference DiffType) {
           return Diag(FirstTD->getLocation(),
bool ODRDiagsEmitter::diagnoseSubMismatchTypedef(const NamedDecl *FirstRecord,
 
         QualType FirstType = FirstTD->getUnderlyingType();
         QualType SecondType = SecondTD->getUnderlyingType();
         if (computeODRHash(FirstType) != computeODRHash(SecondType)) {
           DiagError(TypedefType) << IsTypeAlias << FirstName << FirstType;
           DiagNote(TypedefType) << IsTypeAlias << SecondName << SecondType;
           return true;
         }
-
         return false;
-      };
+      }
 
-  auto ODRDiagVar = [this](NamedDecl *FirstRecord, StringRef FirstModule,
-                           StringRef SecondModule, const VarDecl *FirstVD,
-                           const VarDecl *SecondVD) {
+  bool ODRDiagsEmitter::diagnoseSubMismatchVar(const NamedDecl *FirstRecord,
+                                               StringRef FirstModule,
+                                               StringRef SecondModule,
+                                               const VarDecl *FirstVD,
+                                               const VarDecl *SecondVD) const {
     enum ODRVarDifference {
       VarName,
       VarType,
       VarSingleInitializer,
       VarDifferentInitializer,
       VarConstexpr,
     };
 
bool ODRDiagsEmitter::diagnoseSubMismatchVar(const NamedDecl *FirstRecord,
     QualType FirstType = FirstVD->getType();
     QualType SecondType = SecondVD->getType();
     if (computeODRHash(FirstType) != computeODRHash(SecondType)) {
       DiagError(VarType) << FirstName << FirstType;
       DiagNote(VarType) << SecondName << SecondType;
       return true;
     }
 
-    if (!PP.getLangOpts().CPlusPlus)
+    if (!LangOpts.CPlusPlus)
       return false;
 
     const Expr *FirstInit = FirstVD->getInit();
     const Expr *SecondInit = SecondVD->getInit();
     if ((FirstInit == nullptr) != (SecondInit == nullptr)) {
       DiagError(VarSingleInitializer)
           << FirstName << (FirstInit == nullptr)
           << (FirstInit ? FirstInit->getSourceRange() : SourceRange());
bool ODRDiagsEmitter::diagnoseSubMismatchVar(const NamedDecl *FirstRecord,
     const bool FirstIsConstexpr = FirstVD->isConstexpr();
     const bool SecondIsConstexpr = SecondVD->isConstexpr();
     if (FirstIsConstexpr != SecondIsConstexpr) {
       DiagError(VarConstexpr) << FirstName << FirstIsConstexpr;
       DiagNote(VarConstexpr) << SecondName << SecondIsConstexpr;
       return true;
     }
     return false;
-  };
-
-  using DeclHashes = llvm::SmallVector<std::pair<Decl *, unsigned>, 4>;
-  auto PopulateHashes = [](DeclHashes &Hashes, RecordDecl *Record,
-                           const DeclContext *DC) {
-    for (auto *D : Record->decls()) {
-      if (!ODRHash::isDeclToBeProcessed(D, DC))
-        continue;
-      Hashes.emplace_back(D, computeODRHash(D));
-    }
-  };
 
-  struct DiffResult {
-    Decl *FirstDecl = nullptr, *SecondDecl = nullptr;
-    ODRMismatchDecl FirstDiffType = Other, SecondDiffType = Other;
-  };
-
-  // If there is a diagnoseable difference, FirstDiffType and
-  // SecondDiffType will not be Other and FirstDecl and SecondDecl will be
-  // filled in if not EndOfClass.
-  auto FindTypeDiffs = [](DeclHashes &FirstHashes, DeclHashes &SecondHashes) {
-    auto DifferenceSelector = [](Decl *D) {
+  ODRDiagsEmitter::DiffResult
+  ODRDiagsEmitter::FindTypeDiffs(DeclHashes &FirstHashes,
+                                 DeclHashes &SecondHashes) {
+    auto DifferenceSelector = [](const Decl *D) {
       assert(D && "valid Decl required");
       switch (D->getKind()) {
       default:
         return Other;
       case Decl::AccessSpec:
         switch (D->getAccess()) {
         case AS_public:
           return PublicSpecifer;
while (FirstIt != FirstHashes.end() || SecondIt != SecondHashes.end()) {
 
       DR.FirstDiffType =
           DR.FirstDecl ? DifferenceSelector(DR.FirstDecl) : EndOfClass;
       DR.SecondDiffType =
           DR.SecondDecl ? DifferenceSelector(DR.SecondDecl) : EndOfClass;
       return DR;
     }
     return DR;
-  };
+  }
 
-  // Use this to diagnose that an unexpected Decl was encountered
-  // or no difference was detected. This causes a generic error
-  // message to be emitted.
-  auto DiagnoseODRUnexpected = [this](DiffResult &DR, NamedDecl *FirstRecord,
-                                      StringRef FirstModule,
-                                      NamedDecl *SecondRecord,
-                                      StringRef SecondModule) {
+  void ODRDiagsEmitter::diagnoseSubMismatchUnexpected(
+      DiffResult &DR, const NamedDecl *FirstRecord, StringRef FirstModule,
+      const NamedDecl *SecondRecord, StringRef SecondModule) const {
     Diag(FirstRecord->getLocation(),
          diag::err_module_odr_violation_different_definitions)
         << FirstRecord << FirstModule.empty() << FirstModule;
 
     if (DR.FirstDecl) {
       Diag(DR.FirstDecl->getLocation(), diag::note_first_module_difference)
           << FirstRecord << DR.FirstDecl->getSourceRange();
     }
 
     Diag(SecondRecord->getLocation(),
          diag::note_module_odr_violation_different_definitions)
         << SecondModule;
 
     if (DR.SecondDecl) {
       Diag(DR.SecondDecl->getLocation(), diag::note_second_module_difference)
           << DR.SecondDecl->getSourceRange();
     }
-  };
+  }
 
-  auto DiagnoseODRMismatch = [this](DiffResult &DR, NamedDecl *FirstRecord,
-                                    StringRef FirstModule,
-                                    NamedDecl *SecondRecord,
-                                    StringRef SecondModule) {
+  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) {
         Loc = Tag->getBraceRange().getEnd();
       } else {
         Loc = D->getLocation();
         Range = D->getSourceRange();
       }
       return std::make_pair(Loc, Range);
     };
 
     auto FirstDiagInfo =
         GetMismatchedDeclLoc(FirstRecord, DR.FirstDiffType, DR.FirstDecl);
     Diag(FirstDiagInfo.first, diag::err_module_odr_violation_mismatch_decl)
         << FirstRecord << FirstModule.empty() << FirstModule
         << FirstDiagInfo.second << DR.FirstDiffType;
 
     auto SecondDiagInfo =
         GetMismatchedDeclLoc(SecondRecord, DR.SecondDiffType, DR.SecondDecl);
     Diag(SecondDiagInfo.first, diag::note_module_odr_violation_mismatch_decl)
         << SecondModule << SecondDiagInfo.second << DR.SecondDiffType;
-  };
-
-  // Issue any pending ODR-failure diagnostics.
-  for (auto &Merge : OdrMergeFailures) {
-    // If we've already pointed out a specific problem with this class, don't
-    // bother issuing a general "something's different" diagnostic.
-    if (!DiagnosedOdrMergeFailures.insert(Merge.first).second)
-      continue;
+  }
 
-    bool Diagnosed = false;
-    CXXRecordDecl *FirstRecord = Merge.first;
-    std::string FirstModule = getOwningModuleNameForDiagnostic(FirstRecord);
-    for (auto &RecordPair : Merge.second) {
-      CXXRecordDecl *SecondRecord = RecordPair.first;
+  bool ODRDiagsEmitter::diagnoseMismatch(
+      const CXXRecordDecl *FirstRecord, const CXXRecordDecl *SecondRecord,
+      const struct CXXRecordDecl::DefinitionData *SecondDD) const {
       // Multiple different declarations got merged together; tell the user
       // where they came from.
       if (FirstRecord == SecondRecord)
-        continue;
+        return false;
 
+      std::string FirstModule = getOwningModuleNameForDiagnostic(FirstRecord);
       std::string SecondModule = getOwningModuleNameForDiagnostic(SecondRecord);
 
-      auto *FirstDD = FirstRecord->DefinitionData;
-      auto *SecondDD = RecordPair.second;
-
+      const struct CXXRecordDecl::DefinitionData *FirstDD =
+          FirstRecord->DefinitionData;
       assert(FirstDD && SecondDD && "Definitions without DefinitionData");
 
       // Diagnostics from DefinitionData are emitted here.
       if (FirstDD != SecondDD) {
+        // Keep in sync with err_module_odr_violation_definition_data.
         enum ODRDefinitionDataDifference {
           NumBases,
           NumVBases,
           BaseType,
           BaseVirtual,
           BaseAccess,
         };
-        auto ODRDiagBaseError = [FirstRecord, &FirstModule,
+        auto DiagBaseError = [FirstRecord, &FirstModule,
                                  this](SourceLocation Loc, SourceRange Range,
                                        ODRDefinitionDataDifference DiffType) {
           return Diag(Loc, diag::err_module_odr_violation_definition_data)
                  << FirstRecord << FirstModule.empty() << FirstModule << Range
                  << DiffType;
         };
-        auto ODRDiagBaseNote = [&SecondModule,
+        auto DiagBaseNote = [&SecondModule,
                                 this](SourceLocation Loc, SourceRange Range,
                                       ODRDefinitionDataDifference DiffType) {
           return Diag(Loc, diag::note_module_odr_violation_definition_data)
                  << SecondModule << Range << DiffType;
         };
-        auto GetSourceRange = [](struct CXXRecordDecl::DefinitionData *DD) {
+        auto GetSourceRange = [](const struct CXXRecordDecl::DefinitionData *DD) {
           unsigned NumBases = DD->NumBases;
           if (NumBases == 0) return SourceRange();
           ArrayRef<CXXBaseSpecifier> bases = DD->bases();
           return SourceRange(bases[0].getBeginLoc(),
                              bases[NumBases - 1].getEndLoc());
         };
 
         unsigned FirstNumBases = FirstDD->NumBases;
         unsigned FirstNumVBases = FirstDD->NumVBases;
         unsigned SecondNumBases = SecondDD->NumBases;
         unsigned SecondNumVBases = SecondDD->NumVBases;
         if (FirstNumBases != SecondNumBases) {
-          ODRDiagBaseError(FirstRecord->getLocation(), GetSourceRange(FirstDD),
+          DiagBaseError(FirstRecord->getLocation(), GetSourceRange(FirstDD),
                            NumBases)
               << FirstNumBases;
-          ODRDiagBaseNote(SecondRecord->getLocation(), GetSourceRange(SecondDD),
+          DiagBaseNote(SecondRecord->getLocation(), GetSourceRange(SecondDD),
                           NumBases)
               << SecondNumBases;
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         if (FirstNumVBases != SecondNumVBases) {
-          ODRDiagBaseError(FirstRecord->getLocation(), GetSourceRange(FirstDD),
+          DiagBaseError(FirstRecord->getLocation(), GetSourceRange(FirstDD),
                            NumVBases)
               << FirstNumVBases;
-          ODRDiagBaseNote(SecondRecord->getLocation(), GetSourceRange(SecondDD),
+          DiagBaseNote(SecondRecord->getLocation(), GetSourceRange(SecondDD),
                           NumVBases)
               << SecondNumVBases;
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         ArrayRef<CXXBaseSpecifier> FirstBases = FirstDD->bases();
         ArrayRef<CXXBaseSpecifier> SecondBases = SecondDD->bases();
-        unsigned I = 0;
-        for (I = 0; I < FirstNumBases; ++I) {
+        for (unsigned I = 0; I < FirstNumBases; ++I) {
           const CXXBaseSpecifier FirstBase = FirstBases[I];
           const CXXBaseSpecifier SecondBase = SecondBases[I];
           if (computeODRHash(FirstBase.getType()) !=
               computeODRHash(SecondBase.getType())) {
-            ODRDiagBaseError(FirstRecord->getLocation(),
+            DiagBaseError(FirstRecord->getLocation(),
                              FirstBase.getSourceRange(), BaseType)
                 << (I + 1) << FirstBase.getType();
-            ODRDiagBaseNote(SecondRecord->getLocation(),
+            DiagBaseNote(SecondRecord->getLocation(),
                             SecondBase.getSourceRange(), BaseType)
                 << (I + 1) << SecondBase.getType();
-            break;
+            return true;
           }
 
           if (FirstBase.isVirtual() != SecondBase.isVirtual()) {
-            ODRDiagBaseError(FirstRecord->getLocation(),
+            DiagBaseError(FirstRecord->getLocation(),
                              FirstBase.getSourceRange(), BaseVirtual)
                 << (I + 1) << FirstBase.isVirtual() << FirstBase.getType();
-            ODRDiagBaseNote(SecondRecord->getLocation(),
+            DiagBaseNote(SecondRecord->getLocation(),
                             SecondBase.getSourceRange(), BaseVirtual)
                 << (I + 1) << SecondBase.isVirtual() << SecondBase.getType();
-            break;
+            return true;
           }
 
           if (FirstBase.getAccessSpecifierAsWritten() !=
               SecondBase.getAccessSpecifierAsWritten()) {
-            ODRDiagBaseError(FirstRecord->getLocation(),
+            DiagBaseError(FirstRecord->getLocation(),
                              FirstBase.getSourceRange(), BaseAccess)
                 << (I + 1) << FirstBase.getType()
                 << (int)FirstBase.getAccessSpecifierAsWritten();
-            ODRDiagBaseNote(SecondRecord->getLocation(),
+            DiagBaseNote(SecondRecord->getLocation(),
                             SecondBase.getSourceRange(), BaseAccess)
                 << (I + 1) << SecondBase.getType()
                 << (int)SecondBase.getAccessSpecifierAsWritten();
-            break;
-          }
+            return true;
           }
-
-        if (I != FirstNumBases) {
-          Diagnosed = true;
-          break;
         }
       }
 
       const ClassTemplateDecl *FirstTemplate =
           FirstRecord->getDescribedClassTemplate();
       const ClassTemplateDecl *SecondTemplate =
           SecondRecord->getDescribedClassTemplate();
 
       assert(!FirstTemplate == !SecondTemplate &&
              "Both pointers should be null or non-null");
 
       if (FirstTemplate && SecondTemplate) {
-        DeclHashes FirstTemplateHashes;
-        DeclHashes SecondTemplateHashes;
-
-        auto PopulateTemplateParameterHashs = [](DeclHashes &Hashes,
-                                                 const ClassTemplateDecl *TD) {
-          for (auto *D : TD->getTemplateParameters()->asArray()) {
-            Hashes.emplace_back(D, computeODRHash(D));
-          }
-        };
-
-        PopulateTemplateParameterHashs(FirstTemplateHashes, FirstTemplate);
-        PopulateTemplateParameterHashs(SecondTemplateHashes, SecondTemplate);
-
-        assert(FirstTemplateHashes.size() == SecondTemplateHashes.size() &&
+        ArrayRef<const NamedDecl *> FirstTemplateParams =
+            FirstTemplate->getTemplateParameters()->asArray();
+        ArrayRef<const NamedDecl *> SecondTemplateParams =
+            SecondTemplate->getTemplateParameters()->asArray();
+        assert(FirstTemplateParams.size() == SecondTemplateParams.size() &&
                "Number of template parameters should be equal.");
-
-        auto FirstIt = FirstTemplateHashes.begin();
-        auto FirstEnd = FirstTemplateHashes.end();
-        auto SecondIt = SecondTemplateHashes.begin();
-        for (; FirstIt != FirstEnd; ++FirstIt, ++SecondIt) {
-          if (FirstIt->second == SecondIt->second)
+        for (auto Pair : llvm::zip(FirstTemplateParams, SecondTemplateParams)) {
+          const NamedDecl *FirstDecl = std::get<0>(Pair);
+          const NamedDecl *SecondDecl = std::get<1>(Pair);
+          if (computeODRHash(FirstDecl) == computeODRHash(SecondDecl))
             continue;
 
-          const NamedDecl* FirstDecl = cast<NamedDecl>(FirstIt->first);
-          const NamedDecl* SecondDecl = cast<NamedDecl>(SecondIt->first);
-
           assert(FirstDecl->getKind() == SecondDecl->getKind() &&
                  "Parameter Decl's should be the same kind.");
 
           enum ODRTemplateDifference {
             ParamEmptyName,
             ParamName,
             ParamSingleDefaultArgument,
             ParamDifferentDefaultArgument,
bool ODRDiagsEmitter::diagnoseMismatch(
                 diag::err_module_odr_violation_template_parameter)
               << FirstRecord << FirstModule.empty() << FirstModule
               << FirstDecl->getSourceRange() << ErrDiffType << hasFirstArg
               << FirstName;
           Diag(SecondDecl->getLocation(),
                 diag::note_module_odr_violation_template_parameter)
               << SecondModule << SecondDecl->getSourceRange() << NoteDiffType
               << hasSecondArg << SecondName;
-          break;
+          return true;
         }
-
-        if (FirstIt != FirstEnd) {
-          Diagnosed = true;
-          break;
-        }
+
+      auto PopulateHashes = [](DeclHashes &Hashes, const RecordDecl *Record,
+                               const DeclContext *DC) {
+        for (const Decl *D : Record->decls()) {
+          if (!ODRHash::isDeclToBeProcessed(D, DC))
+            continue;
+          Hashes.emplace_back(D, computeODRHash(D));
         }
+      };
 
       DeclHashes FirstHashes;
       DeclHashes SecondHashes;
       const DeclContext *DC = FirstRecord;
       PopulateHashes(FirstHashes, FirstRecord, DC);
       PopulateHashes(SecondHashes, SecondRecord, DC);
 
       DiffResult DR = FindTypeDiffs(FirstHashes, SecondHashes);
       ODRMismatchDecl FirstDiffType = DR.FirstDiffType;
       ODRMismatchDecl SecondDiffType = DR.SecondDiffType;
       const Decl *FirstDecl = DR.FirstDecl;
       const Decl *SecondDecl = DR.SecondDecl;
 
       if (FirstDiffType == Other || SecondDiffType == Other) {
-        DiagnoseODRUnexpected(DR, FirstRecord, FirstModule, SecondRecord,
+        diagnoseSubMismatchUnexpected(DR, FirstRecord, FirstModule, SecondRecord,
                                       SecondModule);
-        Diagnosed = true;
-        break;
+        return true;
       }
 
       if (FirstDiffType != SecondDiffType) {
-        DiagnoseODRMismatch(DR, FirstRecord, FirstModule, SecondRecord,
-                            SecondModule);
-        Diagnosed = true;
-        break;
+        diagnoseSubMismatchDifferentDeclKinds(DR, FirstRecord, FirstModule,
+                                              SecondRecord, SecondModule);
+        return true;
       }
 
       // Used with err_module_odr_violation_record and
       // note_module_odr_violation_record
       enum ODRCXXRecordDifference {
         StaticAssertCondition,
         StaticAssertMessage,
         StaticAssertOnlyMessage,
bool ODRDiagsEmitter::diagnoseMismatch(
         FunctionTemplateDifferentNumberParameters,
         FunctionTemplateParameterDifferentKind,
         FunctionTemplateParameterName,
         FunctionTemplateParameterSingleDefaultArgument,
         FunctionTemplateParameterDifferentDefaultArgument,
         FunctionTemplateParameterDifferentType,
         FunctionTemplatePackParameter,
       };
-      auto ODRDiagDeclError = [FirstRecord, &FirstModule,
+      auto DiagError = [FirstRecord, &FirstModule,
                                this](SourceLocation Loc, SourceRange Range,
                                      ODRCXXRecordDifference DiffType) {
         return Diag(Loc, diag::err_module_odr_violation_record)
                << FirstRecord << FirstModule.empty() << FirstModule << Range
                << DiffType;
       };
-      auto ODRDiagDeclNote = [&SecondModule,
+      auto DiagNote = [&SecondModule,
                               this](SourceLocation Loc, SourceRange Range,
                                     ODRCXXRecordDifference DiffType) {
         return Diag(Loc, diag::note_module_odr_violation_record)
                << SecondModule << Range << DiffType;
       };
 
       assert(FirstDiffType == SecondDiffType);
       switch (FirstDiffType) {
       case Other:
       case EndOfClass:
       case PublicSpecifer:
       case PrivateSpecifer:
       case ProtectedSpecifer:
         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();
         unsigned FirstODRHash = computeODRHash(FirstExpr);
         unsigned SecondODRHash = computeODRHash(SecondExpr);
         if (FirstODRHash != SecondODRHash) {
-          ODRDiagDeclError(FirstExpr->getBeginLoc(),
+          DiagError(FirstExpr->getBeginLoc(),
                            FirstExpr->getSourceRange(), StaticAssertCondition);
-          ODRDiagDeclNote(SecondExpr->getBeginLoc(),
+          DiagNote(SecondExpr->getBeginLoc(),
                           SecondExpr->getSourceRange(), StaticAssertCondition);
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         const StringLiteral *FirstStr = FirstSA->getMessage();
         const StringLiteral *SecondStr = SecondSA->getMessage();
         assert((FirstStr || SecondStr) && "Both messages cannot be empty");
         if ((FirstStr && !SecondStr) || (!FirstStr && SecondStr)) {
           SourceLocation FirstLoc, SecondLoc;
           SourceRange FirstRange, SecondRange;
           if (FirstStr) {
             FirstLoc = FirstStr->getBeginLoc();
             FirstRange = FirstStr->getSourceRange();
           } else {
             FirstLoc = FirstSA->getBeginLoc();
             FirstRange = FirstSA->getSourceRange();
           }
           if (SecondStr) {
             SecondLoc = SecondStr->getBeginLoc();
             SecondRange = SecondStr->getSourceRange();
           } else {
             SecondLoc = SecondSA->getBeginLoc();
             SecondRange = SecondSA->getSourceRange();
           }
-          ODRDiagDeclError(FirstLoc, FirstRange, StaticAssertOnlyMessage)
+          DiagError(FirstLoc, FirstRange, StaticAssertOnlyMessage)
               << (FirstStr == nullptr);
-          ODRDiagDeclNote(SecondLoc, SecondRange, StaticAssertOnlyMessage)
+          DiagNote(SecondLoc, SecondRange, StaticAssertOnlyMessage)
               << (SecondStr == nullptr);
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         if (FirstStr && SecondStr &&
             FirstStr->getString() != SecondStr->getString()) {
-          ODRDiagDeclError(FirstStr->getBeginLoc(), FirstStr->getSourceRange(),
+          DiagError(FirstStr->getBeginLoc(), FirstStr->getSourceRange(),
                            StaticAssertMessage);
-          ODRDiagDeclNote(SecondStr->getBeginLoc(), SecondStr->getSourceRange(),
+          DiagNote(SecondStr->getBeginLoc(), SecondStr->getSourceRange(),
                           StaticAssertMessage);
-          Diagnosed = true;
-          break;
+          return true;
         }
         break;
       }
       case Field: {
-        Diagnosed = ODRDiagField(FirstRecord, FirstModule, SecondModule,
+        if (diagnoseSubMismatchField(FirstRecord, FirstModule, SecondModule,
                                      cast<FieldDecl>(FirstDecl),
-                                 cast<FieldDecl>(SecondDecl));
+                                     cast<FieldDecl>(SecondDecl)))
+          return true;
         break;
       }
       case CXXMethod: {
         enum {
           DiagMethod,
           DiagConstructor,
           DiagDestructor,
         } FirstMethodType,
             SecondMethodType;
         auto GetMethodTypeForDiagnostics = [](const CXXMethodDecl* D) {
           if (isa<CXXConstructorDecl>(D)) return DiagConstructor;
           if (isa<CXXDestructorDecl>(D)) return DiagDestructor;
           return DiagMethod;
         };
         const CXXMethodDecl *FirstMethod = cast<CXXMethodDecl>(FirstDecl);
         const CXXMethodDecl *SecondMethod = cast<CXXMethodDecl>(SecondDecl);
         FirstMethodType = GetMethodTypeForDiagnostics(FirstMethod);
         SecondMethodType = GetMethodTypeForDiagnostics(SecondMethod);
         DeclarationName FirstName = FirstMethod->getDeclName();
         DeclarationName SecondName = SecondMethod->getDeclName();
-        auto DiagMethodError = [&ODRDiagDeclError, FirstMethod, FirstMethodType,
+        auto DiagMethodError = [&DiagError, FirstMethod, FirstMethodType,
                                 FirstName](ODRCXXRecordDifference DiffType) {
-          return ODRDiagDeclError(FirstMethod->getLocation(),
+          return DiagError(FirstMethod->getLocation(),
                                   FirstMethod->getSourceRange(), DiffType)
                  << FirstMethodType << FirstName;
         };
-        auto DiagMethodNote = [&ODRDiagDeclNote, SecondMethod, SecondMethodType,
+        auto DiagMethodNote = [&DiagNote, SecondMethod, SecondMethodType,
                                SecondName](ODRCXXRecordDifference DiffType) {
-          return ODRDiagDeclNote(SecondMethod->getLocation(),
+          return DiagNote(SecondMethod->getLocation(),
                                  SecondMethod->getSourceRange(), DiffType)
                  << SecondMethodType << SecondName;
         };
 
         if (FirstMethodType != SecondMethodType || FirstName != SecondName) {
           DiagMethodError(MethodName);
           DiagMethodNote(MethodName);
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         const bool FirstDeleted = FirstMethod->isDeletedAsWritten();
         const bool SecondDeleted = SecondMethod->isDeletedAsWritten();
         if (FirstDeleted != SecondDeleted) {
           DiagMethodError(MethodDeleted) << FirstDeleted;
           DiagMethodNote(MethodDeleted) << SecondDeleted;
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         const bool FirstDefaulted = FirstMethod->isExplicitlyDefaulted();
         const bool SecondDefaulted = SecondMethod->isExplicitlyDefaulted();
         if (FirstDefaulted != SecondDefaulted) {
           DiagMethodError(MethodDefaulted) << FirstDefaulted;
           DiagMethodNote(MethodDefaulted) << SecondDefaulted;
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         const bool FirstVirtual = FirstMethod->isVirtualAsWritten();
         const bool SecondVirtual = SecondMethod->isVirtualAsWritten();
         const bool FirstPure = FirstMethod->isPure();
         const bool SecondPure = SecondMethod->isPure();
         if ((FirstVirtual || SecondVirtual) &&
             (FirstVirtual != SecondVirtual || FirstPure != SecondPure)) {
           DiagMethodError(MethodVirtual) << FirstPure << FirstVirtual;
           DiagMethodNote(MethodVirtual) << SecondPure << SecondVirtual;
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         // CXXMethodDecl::isStatic uses the canonical Decl.  With Decl merging,
         // FirstDecl is the canonical Decl of SecondDecl, so the storage
         // class needs to be checked instead.
         StorageClass FirstStorage = FirstMethod->getStorageClass();
         StorageClass SecondStorage = SecondMethod->getStorageClass();
         const bool FirstStatic = FirstStorage == SC_Static;
         const bool SecondStatic = SecondStorage == SC_Static;
         if (FirstStatic != SecondStatic) {
           DiagMethodError(MethodStatic) << FirstStatic;
           DiagMethodNote(MethodStatic) << SecondStatic;
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         const bool FirstVolatile = FirstMethod->isVolatile();
         const bool SecondVolatile = SecondMethod->isVolatile();
         if (FirstVolatile != SecondVolatile) {
           DiagMethodError(MethodVolatile) << FirstVolatile;
           DiagMethodNote(MethodVolatile) << SecondVolatile;
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         const bool FirstConst = FirstMethod->isConst();
         const bool SecondConst = SecondMethod->isConst();
         if (FirstConst != SecondConst) {
           DiagMethodError(MethodConst) << FirstConst;
           DiagMethodNote(MethodConst) << SecondConst;
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         const bool FirstInline = FirstMethod->isInlineSpecified();
         const bool SecondInline = SecondMethod->isInlineSpecified();
         if (FirstInline != SecondInline) {
           DiagMethodError(MethodInline) << FirstInline;
           DiagMethodNote(MethodInline) << SecondInline;
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         const unsigned FirstNumParameters = FirstMethod->param_size();
         const unsigned SecondNumParameters = SecondMethod->param_size();
         if (FirstNumParameters != SecondNumParameters) {
           DiagMethodError(MethodNumberParameters) << FirstNumParameters;
           DiagMethodNote(MethodNumberParameters) << SecondNumParameters;
-          Diagnosed = true;
-          break;
+          return true;
         }
 
-        // Need this status boolean to know when break out of the switch.
-        bool ParameterMismatch = false;
         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) !=
bool ODRDiagsEmitter::diagnoseMismatch(
                     SecondParamType->getAs<DecayedType>()) {
               DiagMethodNote(MethodParameterType)
                   << (I + 1) << SecondParamType << true
                   << ParamDecayedType->getOriginalType();
             } else {
               DiagMethodNote(MethodParameterType)
                   << (I + 1) << SecondParamType << false;
             }
-            ParameterMismatch = true;
-            break;
+            return true;
           }
 
           DeclarationName FirstParamName = FirstParam->getDeclName();
           DeclarationName SecondParamName = SecondParam->getDeclName();
           if (FirstParamName != SecondParamName) {
             DiagMethodError(MethodParameterName) << (I + 1) << FirstParamName;
             DiagMethodNote(MethodParameterName) << (I + 1) << SecondParamName;
-            ParameterMismatch = true;
-            break;
+            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)
                 << (SecondInit ? SecondInit->getSourceRange() : SourceRange());
-            ParameterMismatch = true;
-            break;
+            return true;
           }
 
           if (FirstInit && SecondInit &&
               computeODRHash(FirstInit) != computeODRHash(SecondInit)) {
             DiagMethodError(MethodParameterDifferentDefaultArgument)
                 << (I + 1) << FirstInit->getSourceRange();
             DiagMethodNote(MethodParameterDifferentDefaultArgument)
                 << (I + 1) << SecondInit->getSourceRange();
-            ParameterMismatch = true;
-            break;
-          }
+            return true;
           }
-
-        if (ParameterMismatch) {
-          Diagnosed = true;
-          break;
         }
 
         const TemplateArgumentList *FirstTemplateArgs =
             FirstMethod->getTemplateSpecializationArgs();
         const TemplateArgumentList *SecondTemplateArgs =
             SecondMethod->getTemplateSpecializationArgs();
 
         if ((FirstTemplateArgs && !SecondTemplateArgs) ||
             (!FirstTemplateArgs && SecondTemplateArgs)) {
           DiagMethodError(MethodNoTemplateArguments)
               << (FirstTemplateArgs != nullptr);
           DiagMethodNote(MethodNoTemplateArguments)
               << (SecondTemplateArgs != nullptr);
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         if (FirstTemplateArgs && SecondTemplateArgs) {
           // Remove pack expansions from argument list.
           auto ExpandTemplateArgumentList =
               [](const TemplateArgumentList *TAL) {
                 llvm::SmallVector<const TemplateArgument *, 8> ExpandedList;
                 for (const TemplateArgument &TA : TAL->asArray()) {
bool ODRDiagsEmitter::diagnoseMismatch(
           llvm::SmallVector<const TemplateArgument *, 8> SecondExpandedList =
               ExpandTemplateArgumentList(SecondTemplateArgs);
 
           if (FirstExpandedList.size() != SecondExpandedList.size()) {
             DiagMethodError(MethodDifferentNumberTemplateArguments)
                 << (unsigned)FirstExpandedList.size();
             DiagMethodNote(MethodDifferentNumberTemplateArguments)
                 << (unsigned)SecondExpandedList.size();
-            Diagnosed = true;
-            break;
+            return true;
           }
 
-          bool TemplateArgumentMismatch = false;
           for (unsigned i = 0, e = FirstExpandedList.size(); i != e; ++i) {
             const TemplateArgument &FirstTA = *FirstExpandedList[i],
                                    &SecondTA = *SecondExpandedList[i];
             if (computeODRHash(FirstTA) == computeODRHash(SecondTA)) {
               continue;
             }
 
             DiagMethodError(MethodDifferentTemplateArgument)
                 << FirstTA << i + 1;
             DiagMethodNote(MethodDifferentTemplateArgument)
                 << SecondTA << i + 1;
-            TemplateArgumentMismatch = true;
-            break;
-          }
-
-          if (TemplateArgumentMismatch) {
-            Diagnosed = true;
-            break;
+            return true;
           }
         }
 
         // Compute the hash of the method as if it has no body.
         auto ComputeCXXMethodODRHash = [](const CXXMethodDecl *D) {
           ODRHash Hasher;
           Hasher.AddFunctionDecl(D, true /*SkipBody*/);
           return Hasher.CalculateHash();
         };
 
         // Compare the hash generated to the hash stored.  A difference means
         // that a body was present in the original source.  Due to merging,
         // the standard way of detecting a body will not work.
         const bool HasFirstBody =
             ComputeCXXMethodODRHash(FirstMethod) != FirstMethod->getODRHash();
         const bool HasSecondBody =
             ComputeCXXMethodODRHash(SecondMethod) != SecondMethod->getODRHash();
 
         if (HasFirstBody != HasSecondBody) {
           DiagMethodError(MethodSingleBody) << HasFirstBody;
           DiagMethodNote(MethodSingleBody) << HasSecondBody;
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         if (HasFirstBody && HasSecondBody) {
           DiagMethodError(MethodDifferentBody);
           DiagMethodNote(MethodDifferentBody);
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         break;
       }
+
       case TypeAlias:
       case TypeDef: {
-        Diagnosed = ODRDiagTypeDefOrAlias(
-            FirstRecord, FirstModule, SecondModule,
-            cast<TypedefNameDecl>(FirstDecl), cast<TypedefNameDecl>(SecondDecl),
-            FirstDiffType == TypeAlias);
+        if (diagnoseSubMismatchTypedef(FirstRecord, FirstModule, SecondModule,
+                                       cast<TypedefNameDecl>(FirstDecl),
+                                       cast<TypedefNameDecl>(SecondDecl),
+                                       FirstDiffType == TypeAlias))
+          return true;
         break;
       }
       case Var: {
-        Diagnosed =
-            ODRDiagVar(FirstRecord, FirstModule, SecondModule,
-                       cast<VarDecl>(FirstDecl), cast<VarDecl>(SecondDecl));
+        if (diagnoseSubMismatchVar(FirstRecord, FirstModule, SecondModule,
+                                   cast<VarDecl>(FirstDecl),
+                                   cast<VarDecl>(SecondDecl)))
+          return true;
         break;
       }
       case Friend: {
         const FriendDecl *FirstFriend = cast<FriendDecl>(FirstDecl);
         const FriendDecl *SecondFriend = cast<FriendDecl>(SecondDecl);
 
         const NamedDecl *FirstND = FirstFriend->getFriendDecl();
         const NamedDecl *SecondND = SecondFriend->getFriendDecl();
 
         TypeSourceInfo *FirstTSI = FirstFriend->getFriendType();
         TypeSourceInfo *SecondTSI = SecondFriend->getFriendType();
 
         if (FirstND && SecondND) {
-          ODRDiagDeclError(FirstFriend->getFriendLoc(),
+          DiagError(FirstFriend->getFriendLoc(),
                            FirstFriend->getSourceRange(), FriendFunction)
               << FirstND;
-          ODRDiagDeclNote(SecondFriend->getFriendLoc(),
+          DiagNote(SecondFriend->getFriendLoc(),
                           SecondFriend->getSourceRange(), FriendFunction)
               << SecondND;
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         if (FirstTSI && SecondTSI) {
           QualType FirstFriendType = FirstTSI->getType();
           QualType SecondFriendType = SecondTSI->getType();
           assert(computeODRHash(FirstFriendType) !=
                  computeODRHash(SecondFriendType));
-          ODRDiagDeclError(FirstFriend->getFriendLoc(),
+          DiagError(FirstFriend->getFriendLoc(),
                            FirstFriend->getSourceRange(), FriendType)
               << FirstFriendType;
-          ODRDiagDeclNote(SecondFriend->getFriendLoc(),
+          DiagNote(SecondFriend->getFriendLoc(),
                           SecondFriend->getSourceRange(), FriendType)
               << SecondFriendType;
-          Diagnosed = true;
-          break;
+          return true;
         }
 
-        ODRDiagDeclError(FirstFriend->getFriendLoc(),
+        DiagError(FirstFriend->getFriendLoc(),
                          FirstFriend->getSourceRange(), FriendTypeFunction)
             << (FirstTSI == nullptr);
-        ODRDiagDeclNote(SecondFriend->getFriendLoc(),
+        DiagNote(SecondFriend->getFriendLoc(),
                         SecondFriend->getSourceRange(), FriendTypeFunction)
             << (SecondTSI == nullptr);
-        Diagnosed = true;
-        break;
+        return true;
       }
       case FunctionTemplate: {
         const FunctionTemplateDecl *FirstTemplate =
             cast<FunctionTemplateDecl>(FirstDecl);
         const FunctionTemplateDecl *SecondTemplate =
             cast<FunctionTemplateDecl>(SecondDecl);
 
         TemplateParameterList *FirstTPL =
             FirstTemplate->getTemplateParameters();
         TemplateParameterList *SecondTPL =
             SecondTemplate->getTemplateParameters();
 
-        auto DiagTemplateError = [&ODRDiagDeclError, FirstTemplate](
+        auto DiagTemplateError = [&DiagError, FirstTemplate](
                                      ODRCXXRecordDifference DiffType) {
-          return ODRDiagDeclError(FirstTemplate->getLocation(),
+          return DiagError(FirstTemplate->getLocation(),
                                   FirstTemplate->getSourceRange(), DiffType)
                  << FirstTemplate;
         };
-        auto DiagTemplateNote = [&ODRDiagDeclNote, SecondTemplate](
+        auto DiagTemplateNote = [&DiagNote, SecondTemplate](
                                     ODRCXXRecordDifference DiffType) {
-          return ODRDiagDeclNote(SecondTemplate->getLocation(),
+          return DiagNote(SecondTemplate->getLocation(),
                                  SecondTemplate->getSourceRange(), DiffType)
                  << SecondTemplate;
         };
 
         if (FirstTPL->size() != SecondTPL->size()) {
           DiagTemplateError(FunctionTemplateDifferentNumberParameters)
               << FirstTPL->size();
           DiagTemplateNote(FunctionTemplateDifferentNumberParameters)
               << SecondTPL->size();
-          Diagnosed = true;
-          break;
+          return true;
         }
 
-        bool ParameterMismatch = false;
         for (unsigned i = 0, e = FirstTPL->size(); i != e; ++i) {
           NamedDecl *FirstParam = FirstTPL->getParam(i);
           NamedDecl *SecondParam = SecondTPL->getParam(i);
 
           if (FirstParam->getKind() != SecondParam->getKind()) {
             enum {
               TemplateTypeParameter,
               NonTypeTemplateParameter,
bool ODRDiagsEmitter::diagnoseMismatch(
                   return TemplateTemplateParameter;
               }
             };
 
             DiagTemplateError(FunctionTemplateParameterDifferentKind)
                 << (i + 1) << GetParamType(FirstParam);
             DiagTemplateNote(FunctionTemplateParameterDifferentKind)
                 << (i + 1) << GetParamType(SecondParam);
-            ParameterMismatch = true;
-            break;
+            return true;
           }
 
           if (FirstParam->getName() != SecondParam->getName()) {
             DiagTemplateError(FunctionTemplateParameterName)
                 << (i + 1) << (bool)FirstParam->getIdentifier() << FirstParam;
             DiagTemplateNote(FunctionTemplateParameterName)
                 << (i + 1) << (bool)SecondParam->getIdentifier() << SecondParam;
-            ParameterMismatch = true;
-            break;
+            return true;
           }
 
           if (isa<TemplateTypeParmDecl>(FirstParam) &&
               isa<TemplateTypeParmDecl>(SecondParam)) {
             TemplateTypeParmDecl *FirstTTPD =
                 cast<TemplateTypeParmDecl>(FirstParam);
             TemplateTypeParmDecl *SecondTTPD =
                 cast<TemplateTypeParmDecl>(SecondParam);
             bool HasFirstDefaultArgument =
                 FirstTTPD->hasDefaultArgument() &&
                 !FirstTTPD->defaultArgumentWasInherited();
             bool HasSecondDefaultArgument =
                 SecondTTPD->hasDefaultArgument() &&
                 !SecondTTPD->defaultArgumentWasInherited();
             if (HasFirstDefaultArgument != HasSecondDefaultArgument) {
               DiagTemplateError(FunctionTemplateParameterSingleDefaultArgument)
                   << (i + 1) << HasFirstDefaultArgument;
               DiagTemplateNote(FunctionTemplateParameterSingleDefaultArgument)
                   << (i + 1) << HasSecondDefaultArgument;
-              ParameterMismatch = true;
-              break;
+              return true;
             }
 
             if (HasFirstDefaultArgument && HasSecondDefaultArgument) {
               QualType FirstType = FirstTTPD->getDefaultArgument();
               QualType SecondType = SecondTTPD->getDefaultArgument();
               if (computeODRHash(FirstType) != computeODRHash(SecondType)) {
                 DiagTemplateError(
                     FunctionTemplateParameterDifferentDefaultArgument)
                     << (i + 1) << FirstType;
                 DiagTemplateNote(
                     FunctionTemplateParameterDifferentDefaultArgument)
                     << (i + 1) << SecondType;
-                ParameterMismatch = true;
-                break;
+                return true;
               }
             }
 
             if (FirstTTPD->isParameterPack() !=
                 SecondTTPD->isParameterPack()) {
               DiagTemplateError(FunctionTemplatePackParameter)
                   << (i + 1) << FirstTTPD->isParameterPack();
               DiagTemplateNote(FunctionTemplatePackParameter)
                   << (i + 1) << SecondTTPD->isParameterPack();
-              ParameterMismatch = true;
-              break;
+              return true;
             }
           }
 
           if (isa<TemplateTemplateParmDecl>(FirstParam) &&
               isa<TemplateTemplateParmDecl>(SecondParam)) {
             TemplateTemplateParmDecl *FirstTTPD =
                 cast<TemplateTemplateParmDecl>(FirstParam);
             TemplateTemplateParmDecl *SecondTTPD =
bool ODRDiagsEmitter::diagnoseMismatch(
                 };
 
             if (ComputeTemplateParameterListODRHash(FirstTPL) !=
                 ComputeTemplateParameterListODRHash(SecondTPL)) {
               DiagTemplateError(FunctionTemplateParameterDifferentType)
                   << (i + 1);
               DiagTemplateNote(FunctionTemplateParameterDifferentType)
                   << (i + 1);
-              ParameterMismatch = true;
-              break;
+              return true;
             }
 
             bool HasFirstDefaultArgument =
                 FirstTTPD->hasDefaultArgument() &&
                 !FirstTTPD->defaultArgumentWasInherited();
             bool HasSecondDefaultArgument =
                 SecondTTPD->hasDefaultArgument() &&
                 !SecondTTPD->defaultArgumentWasInherited();
             if (HasFirstDefaultArgument != HasSecondDefaultArgument) {
               DiagTemplateError(FunctionTemplateParameterSingleDefaultArgument)
                   << (i + 1) << HasFirstDefaultArgument;
               DiagTemplateNote(FunctionTemplateParameterSingleDefaultArgument)
                   << (i + 1) << HasSecondDefaultArgument;
-              ParameterMismatch = true;
-              break;
+              return true;
             }
 
             if (HasFirstDefaultArgument && HasSecondDefaultArgument) {
               TemplateArgument FirstTA =
                   FirstTTPD->getDefaultArgument().getArgument();
               TemplateArgument SecondTA =
                   SecondTTPD->getDefaultArgument().getArgument();
               if (computeODRHash(FirstTA) != computeODRHash(SecondTA)) {
                 DiagTemplateError(
                     FunctionTemplateParameterDifferentDefaultArgument)
                     << (i + 1) << FirstTA;
                 DiagTemplateNote(
                     FunctionTemplateParameterDifferentDefaultArgument)
                     << (i + 1) << SecondTA;
-                ParameterMismatch = true;
-                break;
+                return true;
               }
             }
 
             if (FirstTTPD->isParameterPack() !=
                 SecondTTPD->isParameterPack()) {
               DiagTemplateError(FunctionTemplatePackParameter)
                   << (i + 1) << FirstTTPD->isParameterPack();
               DiagTemplateNote(FunctionTemplatePackParameter)
                   << (i + 1) << SecondTTPD->isParameterPack();
-              ParameterMismatch = true;
-              break;
+              return true;
             }
           }
 
           if (isa<NonTypeTemplateParmDecl>(FirstParam) &&
               isa<NonTypeTemplateParmDecl>(SecondParam)) {
             NonTypeTemplateParmDecl *FirstNTTPD =
                 cast<NonTypeTemplateParmDecl>(FirstParam);
             NonTypeTemplateParmDecl *SecondNTTPD =
                 cast<NonTypeTemplateParmDecl>(SecondParam);
 
             QualType FirstType = FirstNTTPD->getType();
             QualType SecondType = SecondNTTPD->getType();
             if (computeODRHash(FirstType) != computeODRHash(SecondType)) {
               DiagTemplateError(FunctionTemplateParameterDifferentType)
                   << (i + 1);
               DiagTemplateNote(FunctionTemplateParameterDifferentType)
                   << (i + 1);
-              ParameterMismatch = true;
-              break;
+              return true;
             }
 
             bool HasFirstDefaultArgument =
                 FirstNTTPD->hasDefaultArgument() &&
                 !FirstNTTPD->defaultArgumentWasInherited();
             bool HasSecondDefaultArgument =
                 SecondNTTPD->hasDefaultArgument() &&
                 !SecondNTTPD->defaultArgumentWasInherited();
             if (HasFirstDefaultArgument != HasSecondDefaultArgument) {
               DiagTemplateError(FunctionTemplateParameterSingleDefaultArgument)
                   << (i + 1) << HasFirstDefaultArgument;
               DiagTemplateNote(FunctionTemplateParameterSingleDefaultArgument)
                   << (i + 1) << HasSecondDefaultArgument;
-              ParameterMismatch = true;
-              break;
+              return true;
             }
 
             if (HasFirstDefaultArgument && HasSecondDefaultArgument) {
               Expr *FirstDefaultArgument = FirstNTTPD->getDefaultArgument();
               Expr *SecondDefaultArgument = SecondNTTPD->getDefaultArgument();
               if (computeODRHash(FirstDefaultArgument) !=
                   computeODRHash(SecondDefaultArgument)) {
                 DiagTemplateError(
                     FunctionTemplateParameterDifferentDefaultArgument)
                     << (i + 1) << FirstDefaultArgument;
                 DiagTemplateNote(
                     FunctionTemplateParameterDifferentDefaultArgument)
                     << (i + 1) << SecondDefaultArgument;
-                ParameterMismatch = true;
-                break;
+                return true;
               }
             }
 
             if (FirstNTTPD->isParameterPack() !=
                 SecondNTTPD->isParameterPack()) {
               DiagTemplateError(FunctionTemplatePackParameter)
                   << (i + 1) << FirstNTTPD->isParameterPack();
               DiagTemplateNote(FunctionTemplatePackParameter)
                   << (i + 1) << SecondNTTPD->isParameterPack();
-              ParameterMismatch = true;
-              break;
-            }
+              return true;
             }
           }
-
-        if (ParameterMismatch) {
-          Diagnosed = true;
-          break;
         }
-
         break;
       }
       }
 
-      if (Diagnosed)
-        continue;
-
       Diag(FirstDecl->getLocation(),
            diag::err_module_odr_violation_mismatch_decl_unknown)
           << FirstRecord << FirstModule.empty() << FirstModule << FirstDiffType
           << FirstDecl->getSourceRange();
       Diag(SecondDecl->getLocation(),
            diag::note_module_odr_violation_mismatch_decl_unknown)
           << SecondModule << FirstDiffType << SecondDecl->getSourceRange();
-      Diagnosed = true;
+      return true;
     }
 
-    if (!Diagnosed) {
-      // All definitions are updates to the same declaration. This happens if a
-      // module instantiates the declaration of a class template specialization
-      // and two or more other modules instantiate its definition.
-      //
-      // FIXME: Indicate which modules had instantiations of this definition.
-      // FIXME: How can this even happen?
-      Diag(Merge.first->getLocation(),
-           diag::err_module_odr_violation_different_instantiations)
-        << Merge.first;
-    }
-  }
+  bool ODRDiagsEmitter::diagnoseMismatch(
+      const FunctionDecl *FirstFunction,
+      const FunctionDecl *SecondFunction) const {
+    if (FirstFunction == SecondFunction)
+      return false;
 
-  // Issue ODR failures diagnostics for functions.
-  for (auto &Merge : FunctionOdrMergeFailures) {
+    // Keep in sync with select options in err_module_odr_violation_function.
     enum ODRFunctionDifference {
       ReturnType,
       ParameterName,
       ParameterType,
       ParameterSingleDefaultArgument,
       ParameterDifferentDefaultArgument,
       FunctionBody,
     };
 
-    FunctionDecl *FirstFunction = Merge.first;
     std::string FirstModule = getOwningModuleNameForDiagnostic(FirstFunction);
+    std::string SecondModule = getOwningModuleNameForDiagnostic(SecondFunction);
 
-    bool Diagnosed = false;
-    for (auto &SecondFunction : Merge.second) {
-
-      if (FirstFunction == SecondFunction)
-        continue;
-
-      std::string SecondModule =
-          getOwningModuleNameForDiagnostic(SecondFunction);
-
-      auto ODRDiagError = [FirstFunction, &FirstModule,
+      auto DiagError = [FirstFunction, &FirstModule,
                            this](SourceLocation Loc, SourceRange Range,
                                  ODRFunctionDifference DiffType) {
         return Diag(Loc, diag::err_module_odr_violation_function)
                << FirstFunction << FirstModule.empty() << FirstModule << Range
                << DiffType;
       };
-      auto ODRDiagNote = [&SecondModule, this](SourceLocation Loc,
+      auto DiagNote = [&SecondModule, this](SourceLocation Loc,
                                                SourceRange Range,
                                                ODRFunctionDifference DiffType) {
         return Diag(Loc, diag::note_module_odr_violation_function)
                << SecondModule << Range << DiffType;
       };
 
       if (computeODRHash(FirstFunction->getReturnType()) !=
           computeODRHash(SecondFunction->getReturnType())) {
-        ODRDiagError(FirstFunction->getReturnTypeSourceRange().getBegin(),
+        DiagError(FirstFunction->getReturnTypeSourceRange().getBegin(),
                      FirstFunction->getReturnTypeSourceRange(), ReturnType)
             << FirstFunction->getReturnType();
-        ODRDiagNote(SecondFunction->getReturnTypeSourceRange().getBegin(),
+        DiagNote(SecondFunction->getReturnTypeSourceRange().getBegin(),
                     SecondFunction->getReturnTypeSourceRange(), ReturnType)
             << SecondFunction->getReturnType();
-        Diagnosed = true;
-        break;
+        return true;
       }
 
       assert(FirstFunction->param_size() == SecondFunction->param_size() &&
              "Merged functions with different number of parameters");
 
       size_t ParamSize = FirstFunction->param_size();
-      bool ParameterMismatch = false;
       for (unsigned I = 0; I < ParamSize; ++I) {
         const ParmVarDecl *FirstParam = FirstFunction->getParamDecl(I);
         const ParmVarDecl *SecondParam = SecondFunction->getParamDecl(I);
 
-        assert(getContext().hasSameType(FirstParam->getType(),
+        assert(Context.hasSameType(FirstParam->getType(),
                                       SecondParam->getType()) &&
                "Merged function has different parameter types.");
 
         if (FirstParam->getDeclName() != SecondParam->getDeclName()) {
-          ODRDiagError(FirstParam->getLocation(), FirstParam->getSourceRange(),
+          DiagError(FirstParam->getLocation(), FirstParam->getSourceRange(),
                        ParameterName)
               << I + 1 << FirstParam->getDeclName();
-          ODRDiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(),
+          DiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(),
                       ParameterName)
               << I + 1 << SecondParam->getDeclName();
-          ParameterMismatch = true;
-          break;
+          return true;
         };
 
         QualType FirstParamType = FirstParam->getType();
         QualType SecondParamType = SecondParam->getType();
         if (FirstParamType != SecondParamType &&
             computeODRHash(FirstParamType) != computeODRHash(SecondParamType)) {
           if (const DecayedType *ParamDecayedType =
                   FirstParamType->getAs<DecayedType>()) {
-            ODRDiagError(FirstParam->getLocation(),
+            DiagError(FirstParam->getLocation(),
                          FirstParam->getSourceRange(), ParameterType)
                 << (I + 1) << FirstParamType << true
                 << ParamDecayedType->getOriginalType();
           } else {
-            ODRDiagError(FirstParam->getLocation(),
+            DiagError(FirstParam->getLocation(),
                          FirstParam->getSourceRange(), ParameterType)
                 << (I + 1) << FirstParamType << false;
           }
 
           if (const DecayedType *ParamDecayedType =
                   SecondParamType->getAs<DecayedType>()) {
-            ODRDiagNote(SecondParam->getLocation(),
+            DiagNote(SecondParam->getLocation(),
                         SecondParam->getSourceRange(), ParameterType)
                 << (I + 1) << SecondParamType << true
                 << ParamDecayedType->getOriginalType();
           } else {
-            ODRDiagNote(SecondParam->getLocation(),
+            DiagNote(SecondParam->getLocation(),
                         SecondParam->getSourceRange(), ParameterType)
                 << (I + 1) << SecondParamType << false;
           }
-          ParameterMismatch = true;
-          break;
+          return true;
         }
 
         const Expr *FirstInit = FirstParam->getInit();
         const Expr *SecondInit = SecondParam->getInit();
         if ((FirstInit == nullptr) != (SecondInit == nullptr)) {
-          ODRDiagError(FirstParam->getLocation(), FirstParam->getSourceRange(),
+          DiagError(FirstParam->getLocation(), FirstParam->getSourceRange(),
                        ParameterSingleDefaultArgument)
               << (I + 1) << (FirstInit == nullptr)
               << (FirstInit ? FirstInit->getSourceRange() : SourceRange());
-          ODRDiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(),
+          DiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(),
                       ParameterSingleDefaultArgument)
               << (I + 1) << (SecondInit == nullptr)
               << (SecondInit ? SecondInit->getSourceRange() : SourceRange());
-          ParameterMismatch = true;
-          break;
+          return true;
         }
 
         if (FirstInit && SecondInit &&
             computeODRHash(FirstInit) != computeODRHash(SecondInit)) {
-          ODRDiagError(FirstParam->getLocation(), FirstParam->getSourceRange(),
+          DiagError(FirstParam->getLocation(), FirstParam->getSourceRange(),
                        ParameterDifferentDefaultArgument)
               << (I + 1) << FirstInit->getSourceRange();
-          ODRDiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(),
+          DiagNote(SecondParam->getLocation(), SecondParam->getSourceRange(),
                       ParameterDifferentDefaultArgument)
               << (I + 1) << SecondInit->getSourceRange();
-          ParameterMismatch = true;
-          break;
+          return true;
         }
 
         assert(computeODRHash(FirstParam) == computeODRHash(SecondParam) &&
                "Undiagnosed parameter difference.");
       }
 
-      if (ParameterMismatch) {
-        Diagnosed = true;
-        break;
-      }
-
       // If no error has been generated before now, assume the problem is in
       // the body and generate a message.
-      ODRDiagError(FirstFunction->getLocation(),
+      DiagError(FirstFunction->getLocation(),
                    FirstFunction->getSourceRange(), FunctionBody);
-      ODRDiagNote(SecondFunction->getLocation(),
+      DiagNote(SecondFunction->getLocation(),
                   SecondFunction->getSourceRange(), FunctionBody);
-      Diagnosed = true;
-      break;
-    }
-    (void)Diagnosed;
-    assert(Diagnosed && "Unable to emit ODR diagnostic.");
+      return true;
   }
 
-  // Issue ODR failures diagnostics for enums.
-  for (auto &Merge : EnumOdrMergeFailures) {
+  bool ODRDiagsEmitter::diagnoseMismatch(const EnumDecl *FirstEnum,
+                                         const EnumDecl *SecondEnum) const {
+    if (FirstEnum == SecondEnum)
+      return false;
+
+    // Keep in sync with select options in err_module_odr_violation_enum.
     enum ODREnumDifference {
       SingleScopedEnum,
       EnumTagKeywordMismatch,
       SingleSpecifiedType,
       DifferentSpecifiedTypes,
       DifferentNumberEnumConstants,
       EnumConstantName,
       EnumConstantSingleInitializer,
       EnumConstantDifferentInitializer,
     };
 
-    // If we've already pointed out a specific problem with this enum, don't
-    // bother issuing a general "something's different" diagnostic.
-    if (!DiagnosedOdrMergeFailures.insert(Merge.first).second)
-      continue;
-
-    EnumDecl *FirstEnum = Merge.first;
     std::string FirstModule = getOwningModuleNameForDiagnostic(FirstEnum);
+    std::string SecondModule = getOwningModuleNameForDiagnostic(SecondEnum);
 
-    using DeclHashes =
-        llvm::SmallVector<std::pair<EnumConstantDecl *, unsigned>, 4>;
-    auto PopulateHashes = [FirstEnum](DeclHashes &Hashes, EnumDecl *Enum) {
-      for (auto *D : Enum->decls()) {
-        // Due to decl merging, the first EnumDecl is the parent of
-        // Decls in both records.
-        if (!ODRHash::isDeclToBeProcessed(D, FirstEnum))
-          continue;
-        assert(isa<EnumConstantDecl>(D) && "Unexpected Decl kind");
-        Hashes.emplace_back(cast<EnumConstantDecl>(D), computeODRHash(D));
-      }
-    };
-    DeclHashes FirstHashes;
-    PopulateHashes(FirstHashes, FirstEnum);
-    bool Diagnosed = false;
-    for (auto &SecondEnum : Merge.second) {
-
-      if (FirstEnum == SecondEnum)
-        continue;
-
-      std::string SecondModule =
-          getOwningModuleNameForDiagnostic(SecondEnum);
-
-      auto ODRDiagError = [FirstEnum, &FirstModule,
+      auto DiagError = [FirstEnum, &FirstModule,
                            this](const auto *DiagAnchor,
                                  ODREnumDifference DiffType) {
         return Diag(DiagAnchor->getLocation(),
                     diag::err_module_odr_violation_enum)
                << FirstEnum << FirstModule.empty() << FirstModule
                << DiagAnchor->getSourceRange() << DiffType;
       };
-      auto ODRDiagNote = [&SecondModule, this](const auto *DiagAnchor,
+      auto DiagNote = [&SecondModule, this](const auto *DiagAnchor,
                                                ODREnumDifference DiffType) {
         return Diag(DiagAnchor->getLocation(),
                     diag::note_module_odr_violation_enum)
                << SecondModule << DiagAnchor->getSourceRange() << DiffType;
       };
 
       if (FirstEnum->isScoped() != SecondEnum->isScoped()) {
-        ODRDiagError(FirstEnum, SingleScopedEnum) << FirstEnum->isScoped();
-        ODRDiagNote(SecondEnum, SingleScopedEnum) << SecondEnum->isScoped();
-        Diagnosed = true;
-        continue;
+        DiagError(FirstEnum, SingleScopedEnum) << FirstEnum->isScoped();
+        DiagNote(SecondEnum, SingleScopedEnum) << SecondEnum->isScoped();
+        return true;
       }
 
       if (FirstEnum->isScoped() && SecondEnum->isScoped()) {
         if (FirstEnum->isScopedUsingClassTag() !=
             SecondEnum->isScopedUsingClassTag()) {
-          ODRDiagError(FirstEnum, EnumTagKeywordMismatch)
+          DiagError(FirstEnum, EnumTagKeywordMismatch)
               << FirstEnum->isScopedUsingClassTag();
-          ODRDiagNote(SecondEnum, EnumTagKeywordMismatch)
+          DiagNote(SecondEnum, EnumTagKeywordMismatch)
               << SecondEnum->isScopedUsingClassTag();
-          Diagnosed = true;
-          continue;
+          return true;
         }
       }
 
       QualType FirstUnderlyingType =
           FirstEnum->getIntegerTypeSourceInfo()
               ? FirstEnum->getIntegerTypeSourceInfo()->getType()
               : QualType();
       QualType SecondUnderlyingType =
           SecondEnum->getIntegerTypeSourceInfo()
               ? SecondEnum->getIntegerTypeSourceInfo()->getType()
               : QualType();
       if (FirstUnderlyingType.isNull() != SecondUnderlyingType.isNull()) {
-        ODRDiagError(FirstEnum, SingleSpecifiedType)
+        DiagError(FirstEnum, SingleSpecifiedType)
             << !FirstUnderlyingType.isNull();
-        ODRDiagNote(SecondEnum, SingleSpecifiedType)
+        DiagNote(SecondEnum, SingleSpecifiedType)
             << !SecondUnderlyingType.isNull();
-        Diagnosed = true;
-        continue;
+        return true;
       }
 
       if (!FirstUnderlyingType.isNull() && !SecondUnderlyingType.isNull()) {
         if (computeODRHash(FirstUnderlyingType) !=
             computeODRHash(SecondUnderlyingType)) {
-          ODRDiagError(FirstEnum, DifferentSpecifiedTypes)
+          DiagError(FirstEnum, DifferentSpecifiedTypes)
               << FirstUnderlyingType;
-          ODRDiagNote(SecondEnum, DifferentSpecifiedTypes)
+          DiagNote(SecondEnum, DifferentSpecifiedTypes)
               << SecondUnderlyingType;
-          Diagnosed = true;
-          continue;
+          return true;
         }
       }
 
+      // Compare enum constants.
+      using DeclHashes =
+          llvm::SmallVector<std::pair<const EnumConstantDecl *, unsigned>, 4>;
+      auto PopulateHashes = [FirstEnum](DeclHashes &Hashes, const EnumDecl *Enum) {
+        for (const Decl *D : Enum->decls()) {
+          // Due to decl merging, the first EnumDecl is the parent of
+          // Decls in both records.
+          if (!ODRHash::isDeclToBeProcessed(D, FirstEnum))
+            continue;
+          assert(isa<EnumConstantDecl>(D) && "Unexpected Decl kind");
+          Hashes.emplace_back(cast<EnumConstantDecl>(D), computeODRHash(D));
+        }
+      };
+      DeclHashes FirstHashes;
+      PopulateHashes(FirstHashes, FirstEnum);
       DeclHashes SecondHashes;
       PopulateHashes(SecondHashes, SecondEnum);
 
       if (FirstHashes.size() != SecondHashes.size()) {
-        ODRDiagError(FirstEnum, DifferentNumberEnumConstants)
+        DiagError(FirstEnum, DifferentNumberEnumConstants)
             << (int)FirstHashes.size();
-        ODRDiagNote(SecondEnum, DifferentNumberEnumConstants)
+        DiagNote(SecondEnum, DifferentNumberEnumConstants)
             << (int)SecondHashes.size();
-        Diagnosed = true;
-        continue;
+        return true;
       }
 
-      for (unsigned I = 0; I < FirstHashes.size(); ++I) {
+      for (unsigned I = 0, N = FirstHashes.size(); I < N; ++I) {
         if (FirstHashes[I].second == SecondHashes[I].second)
           continue;
         const EnumConstantDecl *FirstConstant = FirstHashes[I].first;
         const EnumConstantDecl *SecondConstant = SecondHashes[I].first;
 
         if (FirstConstant->getDeclName() != SecondConstant->getDeclName()) {
-
-          ODRDiagError(FirstConstant, EnumConstantName)
+          DiagError(FirstConstant, EnumConstantName)
               << I + 1 << FirstConstant;
-          ODRDiagNote(SecondConstant, EnumConstantName)
+          DiagNote(SecondConstant, EnumConstantName)
               << I + 1 << SecondConstant;
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         const Expr *FirstInit = FirstConstant->getInitExpr();
         const Expr *SecondInit = SecondConstant->getInitExpr();
         if (!FirstInit && !SecondInit)
           continue;
 
         if (!FirstInit || !SecondInit) {
-          ODRDiagError(FirstConstant, EnumConstantSingleInitializer)
+          DiagError(FirstConstant, EnumConstantSingleInitializer)
               << I + 1 << FirstConstant << (FirstInit != nullptr);
-          ODRDiagNote(SecondConstant, EnumConstantSingleInitializer)
+          DiagNote(SecondConstant, EnumConstantSingleInitializer)
               << I + 1 << SecondConstant << (SecondInit != nullptr);
-          Diagnosed = true;
-          break;
+          return true;
         }
 
         if (computeODRHash(FirstInit) != computeODRHash(SecondInit)) {
-          ODRDiagError(FirstConstant, EnumConstantDifferentInitializer)
+          DiagError(FirstConstant, EnumConstantDifferentInitializer)
               << I + 1 << FirstConstant;
-          ODRDiagNote(SecondConstant, EnumConstantDifferentInitializer)
+          DiagNote(SecondConstant, EnumConstantDifferentInitializer)
               << I + 1 << SecondConstant;
-          Diagnosed = true;
-          break;
-        }
+          return true;
       }
     }
-
-    (void)Diagnosed;
-    assert(Diagnosed && "Unable to emit ODR diagnostic.");
-  }
+    return false;
 }
+// clang-format on
 
 void ASTReader::StartedDeserializing() {
   if (++NumCurrentElementsDeserializing == 1 && ReadTimer.get())
     ReadTimer->startTimer();
 }
 
 void ASTReader::FinishedDeserializing() {
   assert(NumCurrentElementsDeserializing &&