[PATCH] D78126: [analyzer][NFC] Don't allow dependency checkers to emit diagnostics

Fri Jun 12 12:02:46 PDT 2020

Szelethus marked an inline comment as done.
Szelethus added inline comments.

================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:43
 #include "clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
 #include "llvm/ADT/ArrayRef.h"
----------------
saugustine wrote:
> Unfortunately, this include creates a circular dependency from StaticAnalyzer/Core to StaticAnalyzer/Frontend back to StaticAnalyzer/Core.
> 
> I'm hoping to figure out a fix that doesn't involve reverting it, but if you have a quick fix, that would be really great.
Yup, this is a common theme with these changes, the divide in between the analyzer's libraries isn't too well defined.

`CheckerRegistry` definitely belongs to the frontend, and `BugReporter` isn't really moving anywhere either. The registry is responsible for parsing the command line arguments (which checker is enabled, values of checker options, resolving dependencies, etc), but this information is really desired in the rest of the analyzer as well.

I think the obvious (to me, that is) solution would be to move the containers to the Core library, and leave the rest of the logic in the frontend. While I dare claiming that I have great knowledge about this part of the codebase (its usually just me maintaining it), I would be more comfortable doing that without needing to rush the solution :)

An uncomfortable hack would be to simply implement the new functions in the frontend library -- but the former solution better describes where the data CheckerRegistry holds should lie.

I'll revert this. Thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78126