aaron.ballman added inline comments.
================
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");
+ });
----------------
cjdb wrote:
> I'm wondering if I should instead copy `CurrentArtifacts` to a vector and
> sort prior to insertion, rather than in post.
I think this is okay, but might be interesting to see what has better perf.
Given that this is 1) related to issuing diagnostics, and 2) dumping data onto
disk, I am not super concerned about perf for this until we see something show
up in a profiler.
================
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);
----------------
cjdb wrote:
> 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.
Rather than have these functions devise their own diagnostic IDs, should we
make some SARIF-specific notes in the diagnostics system that we can use more
directly? (Might be overkill for the first such note here, but the other
`emitFooLocation()` functions make me think we're going to want this wrapped in
a helper sooner rather than later.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145201/new/
https://reviews.llvm.org/D145201
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
