[PATCH] D97449: [Diagnose] Unify MCContext and LLVMContext diagnosing

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

This still feels a bit messy and I don't totally understand how it all fits together, but I'm super supportive of getting rid of the inline asm diagnostic handler and standardizing on DiagnosticHandler/DiagnosticInfo.



================
Comment at: llvm/include/llvm/MC/MCContext.h:33
 #include "llvm/Support/MD5.h"
+#include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/raw_ostream.h"
----------------
MCContext.h is popular, let's try not to leak SourceMgr.h as an include. It probably brings in tons of FS headers that most files don't need.


================
Comment at: llvm/include/llvm/MC/MCContext.h:76
+        std::function<void(const SMDiagnostic &, bool, const SourceMgr &,
+                           std::vector<const MDNode *> &)>;
 
----------------
It doesn't feel right to use MDNode, an IR type, from MC. Is there a way to make this opaque?


================
Comment at: llvm/include/llvm/MC/MCContext.h:386
+      if (!InlineSrcMgr)
+        InlineSrcMgr.reset(new SourceMgr());
+    }
----------------
This would need to be out of line to get by with a forward decl of SourceMgr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97449