vaibhav.y added inline comments.

================
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:
> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to