[PATCH] D55792: Allow direct navigation to static analysis checker documentation through SARIF exports

Kristóf Umann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 18 04:42:30 PST 2018

Szelethus added a comment.

Thank you so much for working on this! The lack of a proper documentation is indeed a weak points of the project.

I guess it'd be better to generate the entire URI in the tblgen file, and also allow plugins to register their own through `CheckerRegistry`. Sarif lacking the support for them is one thing, but adding a new field to the `Checker` class in `CheckerBase.td` I think should be accompanied by extending `CheckerRegistry::CheckerInfo` and `CheckerRegistry::getChecker`.

While I dislike the use of `<a id="some.Checker"></a>`, until Sphinx documentation becomes a thing, I guess we can't do much about it.

But the overall direction of this patch is great.

Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:270-275
+#define CHECKER(FULLNAME, CLASS, HELPTEXT, DOCS)                               \
+#include "clang/StaticAnalyzer/Checkers/Checkers.inc"
+#undef CHECKER
As per usual, this won't handle plugin checkers, but the solution is in an arms reach -- in the last couple months, I changed a lot around checker registration, and once things settle down a bit.

One idea is to store the list of registered checkers (now, in this case, registered means that the analyzer even knows about its existence, not that it's enabled) in `AnalyzerOptions`, however it'd force you to put an extra parameter to almost all functions here. What do you think?

Comment at: lib/StaticAnalyzer/Core/SarifDiagnostics.cpp:284-286
+  return (llvm::Twine("https://clang-analyzer.llvm.org/") + DocLoc + "#" +
+          CheckName)
+      .str();
Why not do this in `ClangSACheckersEmitter.cpp`?

Comment at: utils/TableGen/ClangSACheckersEmitter.cpp:90-91
+  OS << "// This file is automatically generated. Do not edit this file by "
+        "hand.\n";



More information about the cfe-commits mailing list