NoQ 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:
----------------
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).
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
