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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Precommit CI found a relevant failure.



================
Comment at: clang/lib/Frontend/SARIFDiagnosticPrinter.cpp:71
+  // other infrastructure necessary when emitting more rich diagnostics.
+  if (!Info.getLocation().isValid()) { // TODO: What is this case? 
+    // SARIFDiag->addDiagnosticWithoutLocation(
----------------
vaibhav.y wrote:
> vaibhav.y wrote:
> > The location maybe if the diagnostic's source is located in the scratch buffer. Likely for macro expansions where token pasting is involved. Another case would be errors on the command line.
> > 
> > I'm not entirely sure how the SARIF spec would handle this case, it might require an extension.
> > 
> > A few ways that might work could be:
> > 
> > Using the [[ https://docs.oasis-open.org/sarif/sarif/v2.0/csprd02/sarif-v2.0-csprd02.html#_Toc10127692 | logicalLocations ]] property to specify ([[ https://docs.oasis-open.org/sarif/sarif/v2.0/csprd02/sarif-v2.0-csprd02.html#_Toc10127910 | logicalLocation object ]]), this might need an extension for kind: "macro", another case that might need extension is diagnostics about invalid command line flags which are also diagnostics without a valid
> > 
> > The parentIndex for these logical locations could be set to the physical location that produced them.
> > 
> > I think this definitely warrants some discussion since the spec doesn't provide a clear way to express these cases.
> > 
> > WDYT @aaron.ballman @cjdb @denik 
> The spec does say for "kind":
> 
> > If none of those strings accurately describes the construct, kind MAY contain any value specified by the analysis tool.
> 
> So an extension might not be necessary, but might be worth discussing.
>From looking through the spec, I think `logicalLocations` is probably the right choice and we'd want to make up our own kind for things like the scratch buffer or the command line. I think an extension would be worth discussing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632