[PATCH] D75271: [Analyzer][NFC] Add AnalyzerOptions parameter to shouldRegisterXXX() functions

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D75271#1895949 <https://reviews.llvm.org/D75271#1895949>, @baloghadamsoftware wrote:

> It is impossible to use `CheckerManager` as parameter for `shouldRegisterXXX()` because `CheckerRegistry` does not have it. If I add `CheckerManager` to `CheckerRegistry` then the `printXXX()` functions will not work because they do not have `CheckerManager` at all. This patch does not help in printing error message, see D75171 <https://reviews.llvm.org/D75171>. I need a way to solve 44998 <https://bugs.llvm.org/show_bug.cgi?id=44998> by rejecting the checker if the option is disabled **and** printing an error message.


Aha, I see as of D75171 <https://reviews.llvm.org/D75171>. Well, @Szelethus felt the same to pass around the `CheckerManager`. Let us see:

  std::unique_ptr<CheckerManager> ento::createCheckerManager(
      ASTContext &context,                                     
      AnalyzerOptions &opts,
      ArrayRef<std::string> plugins,
      ArrayRef<std::function<void(CheckerRegistry &)>> checkerRegistrationFns,
      DiagnosticsEngine &diags) {                                               
    auto checkerMgr = std::make_unique<CheckerManager>(context, opts);
                                                                        
    CheckerRegistry allCheckers(plugins, diags, opts, context.getLangOpts(),
                                checkerRegistrationFns);                      
                                                          
    allCheckers.initializeManager(*checkerMgr);
    allCheckers.validateCheckerOptions();        
    checkerMgr->finishedCheckerRegistration();
                                                
    return checkerMgr;
  }

- from `StaticAnalyzer/Frontend/CheckerRegistration.cpp`.

Are you sure we cannot pass around the manager like `allCheckers.initializeManager(*checkerMgr);`? I am also thinking of a glue-layer between the two managers, like `CheckerRegistryManager` or something, which holds all your needs, like `AnalyzerOptions` and `LangOptions` for now.

I think if we ask @Szelethus kindly to design his callback using `CheckerManager` instead, we could simplify this patch a lot. As he is the code owner, most likely he continues his journey to make sure people enable the checker dependencies, most likely using `shouldRegisterXXX()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75271