aaron.ballman added a comment.
(Note, precommit CI on Windows still shows a valid failure.)
================
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:
> > 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).
We discussed this offline a bit and yeah, we're going to have to come up with
some sort of naming scheme for these eventually. For the moment, -1 is fine for
this one use case, but you should probably add a FIXME comment about needing to
replace the -1 with something better in the future.
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
