[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Denis Nikitin via Phabricator via cfe-commits]

denik accepted this revision.
denik added inline comments.


================
Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:58
+
+  if (Loc.isValid())
+    Result = addLocationToResult(Result, Loc, PLoc, Ranges, *Diag);
----------------
abrahamcd wrote:
> denik wrote:
> > I think we should add a test case when Loc or/and Source Manager is not set.
> > If Loc is not set SarifDocumentWriter might not create the `locations` property which is required by the spec.
> > If this is the case we need to fix it. But I'm not sure if emitDiagnosticMessage() is going to be called when Loc.isInvalid() ).
> I believe if Loc is invalid, it goes to one of those special cases I mentioned in this review.
If it does then "if" is redundant here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632