[PATCH] D67422: [analyzer] NFC: Move path diagnostic consumer implementations to libAnalysis.

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

Szelethus added a comment.

The patch looks great. I guess the only remaining discussion remains as to whether this should be in `libAnalysis`, or somewhere else. Here is my take: Since clang-tidy, CSA, and some other parts of the compiler (like uninitialized variable warnings) are static analyzers within the same infrastructure, it makes sense for them to have a common library. I see that diagnostics aren't really analyses. `libFrontend` or `libDriver`, which, as I understand it contains most the diagnostics machinery for clang aren't viable alternatives because of the library dependencies. So, I think if `libAnalysis` was called `libStaticAnalysisCommon`, we would be golden, but that would screw over downstream developers for negligible gain. While I'm not terribly experiences in library layout design, for the time being, the move to `libAnalysis` seems appropriate.

I see that we're still heavily tied to StaticAnalyzer headers. Doesn't that violate the module system, as `libAnalysis` is a dependency of `libStaticAnalyzerCore`? `AnalyzerOptions` and `AnalysisConsumer` (that last one in particular) seem deeply integrated into these consumers.



================
Comment at: clang/lib/Analysis/PlistHTMLPathDiagnosticConsumer.cpp:27-31
+  createHTMLDiagnosticConsumer(
+      DiagOpts, C, std::string(llvm::sys::path::parent_path(Prefix)), PP, CTU);
+  createPlistMultiFileDiagnosticConsumer(DiagOpts, C, Prefix, PP, CTU);
+  createTextMinimalPathDiagnosticConsumer(std::move(DiagOpts), C, Prefix, PP,
+                                          CTU);
----------------
NoQ wrote:
> I just realized that this code creates 5 consumers: 1 html consumer, 1 plist consumer, 3 text consumers. This isn't a regression due to this patch... but damn!
Oh wow. That is definitely my mistake. The issue is that detailed diagnostics (`-analyzer-output=text`) and the minimal one (`analyzer-output=text-minimal`) should be mutually exclusive. I'll try to think about a solution. Maybe some `Profile` thing might work.


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

https://reviews.llvm.org/D67422