steakhal added inline comments.
================
Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:124
/// Note that this class also implements caching.
-class CrossTranslationUnitContext {
+class CrossTranslationUnitContext : public CrossTUAnalysisHelper {
public:
----------------
NoQ wrote:
> steakhal wrote:
> > martong wrote:
> > > Why don't we have a dependency in libCrossTU to libAnalysis? In the
> > > CMakeLists.txt?
> > > Here we implement the CrossTUAnalysisHelper's abstract virtual function
> > > thus we include the `CrossTUAnalysisHelper.h`. But
> > > CMake should know about the dependency even if this is only a header only
> > > dependency, shouldn't it? (We were talking about this with @steakhal.)
> > IMO if a library (A) does not depend on another library (B), that means we
> > can safely delete all files of the **B** and still be able to compile and
> > run the **A**.
> > In this case, it won't, as the given header lives under **B**. So to make
> > sure this principle works, we should state the dependency on **B** in **A**.
> >
> > ---
> > In our case, it means that **libCrossTU** depends on **libAnalysis**, as
> > the `CrossTUAnalysisHelper` lives in `libAnalysis` which is used by
> > `libCrossTU` to implement the `CrossTranslationUnitContext`.
> There's already an indirect dependency (you have to look at all those
> CMakeLists.txt in order to notice it): libCrossTU -> libFrontend -> libSema
> -> libAnalysis (and a few other paths through the dependency graph). There's
> no harm in writing it down explicitly but there's also no explicit need and
> i'd rather have it not specified directly (if there's too little dependencies
> it'll warn us) than to specify too much by accident (there's no warning for
> unused dependencies and if a dependency eventually becomes unused it may
> accidentally prevent people from adding the dependencies they actually need).
Thank you for the explanation.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92432/new/
https://reviews.llvm.org/D92432
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
