[PATCH] D90746: [clangd] Cleanup dependencies around RemoteIndex

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 4 05:00:09 PST 2020


kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/CMakeLists.txt:115
   LINK_LIBS
+  clangdRemoteIndex
   clangdSupport
----------------
kbobyrev wrote:
> If we're moving `clangdRemoteIndex` to `clangDaemon` there is no need to link `clangd` itself to `clangdRemoteIndex` anymore (in `tool/CMakeLists.txt`).
All of the libraries linked to clangDeamon are explicitly being linked again to clangd. Even though the ones coming from clang and clangTooling are marked as private here, others like clangdSupport and clangTidy are linked with "default" (whatever that means) visibility. Hence I kept the linking on the clangd side as well.

I suppose moving these to PRIVATE libraries and keeping the extra linking in ClangdMain is the more consistent thing. As ClangdMain still calls `remote::getClient`. Let me know if you disagree or I am missing something.


================
Comment at: clang-tools-extra/clangd/index/remote/CMakeLists.txt:30
     RemoteIndexServiceProto
-    )
 
----------------
kbobyrev wrote:
> I think changing it is inconsistent with LLVM CMake style. Having closing paren on the same indentation level as the items is awkward but that's what we do :(
*sigh*


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90746



More information about the cfe-commits mailing list