[PATCH] D145201: [clang] fixes header processing for `-fdiagnostics-format=sarif`
Christopher Di Bella via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 3 10:56:39 PST 2023
cjdb added a comment.
That test is kinda problematic because it seems that the artifacts aren't ordered. I think we should change this from a
================
Comment at: clang/lib/Basic/Sarif.cpp:314-317
+ llvm::sort(*Artifacts, [](const json::Value &x, const json::Value &y) {
+ return x.getAsObject()->getNumber("index") <
+ y.getAsObject()->getNumber("index");
+ });
----------------
I'm wondering if I should instead copy `CurrentArtifacts` to a vector and sort prior to insertion, rather than in post.
================
Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:214
void SARIFDiagnostic::emitIncludeLocation(FullSourceLoc Loc, PresumedLoc PLoc) {
- assert(false && "Not implemented in SARIF mode");
+ SarifRule Rule = SarifRule::create().setRuleId(std::to_string(-1));
+ Rule = addDiagnosticLevelToRule(Rule, DiagnosticsEngine::Level::Note);
----------------
aaron.ballman wrote:
> Why do we want -1 as the rule ID and... can we use `"-1"` instead of doing a string conversion?
lol at obvious C++ goof.
Re -1, there doesn't seem to be a diagnostic associated with this note, so I picked a value that I know isn't in use.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145201/new/
https://reviews.llvm.org/D145201
More information about the cfe-commits
mailing list