[PATCH] D92432: [analyzer] Add a thin abstraction layer between libCrossTU and libAnalysis.

Artem Dergachev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 1 14:14:07 PST 2020


NoQ created this revision.
NoQ added reviewers: vsavchenko, xazax.hun, baloghadamsoftware, Charusso, martong, balazske, gamesh411.
Herald added subscribers: steakhal, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet.
NoQ requested review of this revision.

After like ~three reverts of D67422 <https://reviews.llvm.org/D67422>, the last known problem with that patch is a circular dependency that that patch introduces: libCrossTU -> libFrontend -> libSema -> libAnalysis -> libCrossTU. Namely, D67422 <https://reviews.llvm.org/D67422> causes libAnalysis to depend on libCrossTU due to `PlistDiagnosticConsumer` (which is moved into libAnalysis in that patch) potentially requiring cross-TU macro expansion information to bundle with the plist.

Circular dependencies are bad because they're not allowed under `-DBUILD_SHARED_LIBS`. It's likely that this problem can be resolved purely on cmake side and i simply wasn't trying hard enough to find a solution (on the other hand, say, `libTooling` had to deal with this by introducing `libToolingCore` - a smaller sub-library with less dependencies).

But, anyway, given that the dependency is extremely tiny, i suggest resolving it by adding a tiny abstraction layer that'd eliminate the dependency entirely. Namely, i'd like to add a virtual interface under `CrossTranslationUnitContext`. Such interface is defined in `libAnalysis` and implemented by the `CrossTranslationUnitContext` class in `libCrossTU`. This vaguely makes sense because libAnalysis is a low-level collection of analysis algorithms whereas libCrossTU is high-level, even frontend-level management of entire clang invocations so calling from libAnalysis to libCrossTU feels like layering violation.

In this patch i add only one function to the interface. If you guys want i can move all (or at least some) public methods of `CrossTranslationUnitContext` into that interface but there's no immediate need for that.


https://reviews.llvm.org/D92432

Files:
  clang/include/clang/Analysis/CrossTUAnalysisHelper.h
  clang/include/clang/Analysis/PathDiagnosticConsumers.h
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/lib/Analysis/HTMLPathDiagnosticConsumer.cpp
  clang/lib/Analysis/PlistHTMLPathDiagnosticConsumer.cpp
  clang/lib/Analysis/PlistPathDiagnosticConsumer.cpp
  clang/lib/Analysis/SarifPathDiagnosticConsumer.cpp
  clang/lib/Analysis/TextPathDiagnosticConsumer.cpp
  clang/lib/CrossTU/CrossTranslationUnit.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D92432.308759.patch
Type: text/x-patch
Size: 13840 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20201201/e94db528/attachment-0001.bin>


More information about the cfe-commits mailing list