cjdb added inline comments.

================
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:
> 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.)
Since it's entirely string-based, we can probably evolve a naming scheme for 
these over time. The numbers are currently useless, so I'm okay with having 
`"-1"` for now though. We should come up with a strategy on whether or not we 
want Microsoft-like codes, but I'm pretty against that (and I think my opinions 
are on the record, but happy to reiterate).


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