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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to