[PATCH] D93223: [RFC][analyzer] Create MacroExpansionContext member in AnalysisConsumer and pass down to the diagnostics consumers

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

steakhal added a comment.

Thanks for the review.



================
Comment at: clang/include/clang/Analysis/PathDiagnostic.h:911
 
+void createHTMLSingleFileDiagnosticConsumer(
+    PathDiagnosticConsumerOptions DiagOpts,
----------------
martong wrote:
> Why do we need this prototype here?
Thanks. I probably accidentally let it there :D I will remove this.
I had a hard time figuring out where and how to modify the signature of this.


================
Comment at: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:129
+        Plugins(plugins), Injector(injector), CTU(CI),
+        MacroExpansions(PP, CI.getLangOpts()) {
     DigestAnalyzerOptions();
----------------
xazax.hun wrote:
> This will always record macro related information right? Some of the diagnostic consumers might not ever touch macro expansions. I think it would be great to only record expansions when we actually will end up using them.  Alternatively, a benchmark proving the runtime of recording negligible on macro heavy code (boost maybe?) might be sufficient.
To be fair, we already have an analyzer config exactly for this.
If I would introduce a function like `registerForPreprocessor(PP)`, the constructor would be freed up to be essentially a noop.
I think I will follow this path.
This approach would fit nicely in the final patch, where I want to put them in a map. Default constructability would be a nice property.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93223