[PATCH] D75842: [Analyzer] Bugfix for CheckerRegistry

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.


================
Comment at: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp:85
+//===----------------------------------------------------------------------===//
+// Unfulfilled dependency
+//===----------------------------------------------------------------------===//
----------------
How about `Unsatisfied checker dependency.`?


================
Comment at: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp:92
+                        BugReporter &BR) const {
+    assert(false && "This checker should not be registered.");
+    BR.EmitBasicReport(D, this, "Prerequisite", categories::LogicError,
----------------
Isn't this overkill? The test above test only the output, but don't outright cause a crash.


================
Comment at: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp:111-112
+                        BugReporter &BR) const {
+    assert(false &&
+           "This checker should not be registered due to missing dependency.");
+    BR.EmitBasicReport(D, this, "Dependent", categories::LogicError,
----------------
Same thing.


================
Comment at: clang/unittests/StaticAnalyzer/RegisterCustomCheckersTest.cpp:143
+  std::string Diags;
+  EXPECT_TRUE(runCheckerOnCode<addDependentChecker>("void f() {;}", Diags));
+}
----------------
I don't think this is checking what you're looking for -- the test should be whether `Diag` is an empty string, while `runCheckerOnCode` returns true when the tool (the static analyzer, in this case) terminates successfully, even if it doesn't work the way we expect it to.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75842/new/

https://reviews.llvm.org/D75842