NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.
Yup, looks good, and i keep passively cheering for this project to be
successful.
================
Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:129
+ unsigned Index = 0;
+ for (const json::Value &File : Files) {
+ if (const json::Object *Obj = File.getAsObject()) {
----------------
This sounds like `find_if` to me.
================
Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:143
+ // that an empty file lists always causes a file to be added.
+ if (Files.empty() || Index == Files.size())
+ Files.push_back(createFile(FE));
----------------
I suspect that the LHS of `||` implies the RHS of `||` and is therefore
redundant.
================
Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:289
+ if (P.second) {
+ RuleMapping[RuleID] = Rules.size(); // Maps RuleID to an Array Index.
+ Rules.push_back(createRule(*D));
----------------
Aha, ok, so now instead of an alphabetical order we have the order in which
reports of the respective checkers are squeezed into the consumer. Which is
probably deterministic, so it's fine. I just enjoy talking to myself on
phabricator sometimes.
================
Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:318
void SarifDiagnostics::FlushDiagnosticsImpl(
std::vector<const PathDiagnostic *> &Diags, FilesMade *) {
// We currently overwrite the file if it already exists. However, it may be
----------------
I wonder why we didn't mark `Diags` as `const &` in this callback.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55707/new/
https://reviews.llvm.org/D55707
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
