[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:143
+  std::string Diags;
+  EXPECT_TRUE(runCheckerOnCode<addDependentChecker>("void f() {;}", Diags));
+}
----------------
baloghadamsoftware wrote:
> Szelethus wrote:
> > 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.
> There could be hundreds of alternative approaches, but this test exactly simulates the real-world problem: the checker crashes because it should not be registered. Of course, I tried the test without the bugfix and it fails as it should because the tool terminates unsuccessfully if the prerequisite checker is registered.
This is still confusing. Please check the string, that should contain what you need and nothing else, and the asserts could be removed as a result -- it shouldn't be more then 5 lines.


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

https://reviews.llvm.org/D75842