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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
