[PATCH] D154478: [analyzer][NFC] Use unique_ptrs for PathDiagnosticConsumers

[Balázs Benics via Phabricator via cfe-commits]

steakhal abandoned this revision.
steakhal added a comment.

Okay, I'm done. It's just a complete mess.
I'll come back to this once I have some time, but not now.

Here is what I found:
Unittests call `AnalysisConsumer->addDiagnosticConsumer()` after the `AnalysisConsumer` is constructed, but before `AnalysisConsumer::Initialize(ASTContext &Context)` is called - because the AST consumer is not yet invoked.
The `AnalysisConsumer::Initialize(ASTContext &Context)` supposed to construct the `CheckerManager` and `AnalysisManager` using the given `ASTContext`.

The `ASTContext` would be available even at the construction of `AnalysisConsumer` via the `CompilerInstance` - and that should work. So, I moved this lazy initialization to the ctor init list. - Works great, however, `CheckerManager` will do some checking for the checker dependencies, hence it needs all checkers to be configured at that point, which means that we should remove `AnalysisConsumer::AddCheckerRegistrationFn()` to make that happen. This implies that the construction of `AnalysisConsumer` would need to have all the checkers we need to configure upfront.
It's likely we can make it work, but would require some engineering on the unittests, which could this patch pump up even more.
And, I'd say, it's likely we can do something better overall if we just step back and redesign how things are getting constructed etc. And avoiding lazy initialization altogether.

I have a pretty good understanding of how things work now, so I might come back once I feel empowered.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154478