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