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