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

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

Reply via email to