vaibhav.y added inline comments.
================
Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:155
+ break;
+ }
+}
----------------
Does this need an `llvm_unreachable` after the switch?
================
Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:177
+ // original path as possible, because that helps a user to recognize it.
+ // real_path() expands all links, which sometimes too much. Luckily,
+ // on Windows we can just use llvm::sys::path::remove_dots(), because,
----------------
================
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(
----------------
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
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
