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

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

Reply via email to