[PATCH] D93224: [RFC][analyzer] Use the MacroExpansionContext for macro expansions in plists

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

steakhal added a comment.

Updates:

- Rebased.

---

Unfortunately, I could not come up with a proper CTU implementation.
It seems that when we load the AST/dump, no preprocessor events are replayed.
Without those events, my `PPCallbacks` implementation and tokenwatcher would not record anything, drawing macro expansion useless for CTU.

How should I continue to get this working with CTU?
@xazax.hun @martong @balazske 
Previously we had some sort of macro expansions (and crashes), after these patches we would not anymore :(



================
Comment at: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:20-21
 #include "clang/Basic/Version.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Lex/Preprocessor.h"
----------------
NoQ wrote:
> steakhal wrote:
> > NoQ wrote:
> > > Will these two includes be ultimately removed? Like, even in the CTU case?
> > I think `PlistDiagnostics` can depend on `CTU`.
> > `ASTUnit` is a different thing though. I think I just accidentally left that include.
> > 
> > I will remove the `#include "clang/Frontend/ASTUnit.h"` line.
> > 
> > ---
> > What do you mean by the 'CTU case'?
> > I think PlistDiagnostics can depend on CTU.
> 
> So, like, my problem is that i want to move all `PathDiagnosticConsumer`s (with implementations) into `libAnalysis` so that other tools could use it but `libAnalysis` can't link-time-depend on `libCrossTU` because that'd be a circular dependency. Basically `libAnalysis` is a tiny library that contains analyses whereas `libCrossTU` is a high-level library that manages entire clang invocations. Unless we rethink our library hierarchies entirely, i believe we're stuck with this constraint.
> 
> Removing the dependency on `libFrontend` is insufficient because `libCrossTU` depends on `libFrontend` anyway. 
> 
> If you make `MacroExpansionContext` abstract and put the concrete implementation in `libCrossTU`, thus replacing my (currently reverted) attempt in D92432 to break the dependency, that'd fix the problem entirely. Otherwise i'll probably wait for your work to land and do this myself anyway.
> 
> > What do you mean by the 'CTU case'?
> 
> Nothing really (: It was an attempt to emotionally highlight the importance of not having a link-time dependency on `libCrossTU` even while handling the CTU execution path, according to whatever definition of that you were alluding to in the review summary.
After you land D92432, these two includes could be substituted by `clang/Analysis/CrossTUAnalysisHelper.h`.


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

https://reviews.llvm.org/D93224