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

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.


================
Comment at: clang/lib/Serialization/ASTReader.cpp:9577-9584
+static std::string 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 {};
----------------
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?


================
Comment at: clang/lib/Serialization/ASTReader.cpp:9733-9734
+    bool Diagnosed = false;
+    ODRDiagsEmitter DiagsEmitter(Diags, getContext(),
+                                 getPreprocessor().getLangOpts());
+    CXXRecordDecl *FirstRecord = Merge.first;
----------------
Could we hoist these construction?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128490/new/

https://reviews.llvm.org/D128490