[clangd] Cleanup dependencies around RemoteIndex

[kadircet]

Summary

RemoteIndexClient implementations only depends on clangdSupport for
logging functionality and has no dependence on clangDeamon itself. This clears
out that link time dependency and enables depending on it in clangDeamon itself,
so that we can have other index implementations that makes use of the
RemoteIndex.

[kbobyrev]

clang-tools-extra/clangd/CMakeLists.txt

114

If we're moving clangdRemoteIndex to clangDaemon there is no need to link clangd itself to clangdRemoteIndex anymore (in tool/CMakeLists.txt).

clang-tools-extra/clangd/index/remote/CMakeLists.txt

30

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 :(

clang-tools-extra/clangd/index/remote/unimplemented/CMakeLists.txt

11

same here

[kadircet]

• Revert indentation changes
• Move all libraries to private to be consistent on what we link in ClangdMain.

clang-tools-extra/clangd/CMakeLists.txt

114

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.

clang-tools-extra/clangd/index/remote/CMakeLists.txt

30

*sigh*

[kbobyrev]

LGTM, thanks!

clang-tools-extra/clangd/CMakeLists.txt

114

I see, makes sense, thanks!

clang-tools-extra/clangd/CMakeLists.txt

 )
 
 set(LLVM_LINK_COMPONENTS
   Support
   AllTargetsInfos
   FrontendOpenMP
   Option
   )
-  
 include(${CMAKE_CURRENT_SOURCE_DIR}/quality/CompletionModel.cmake)
 gen_decision_forest(${CMAKE_CURRENT_SOURCE_DIR}/quality/model CompletionModel clang::clangd::Example)
 
 if(MSVC AND NOT CLANG_CL)
  set_source_files_properties(CompileCommands.cpp PROPERTIES COMPILE_FLAGS -wd4130) # disables C4130: logical operation on address of string constant
 endif()
 
 include_directories(BEFORE "${CMAKE_CURRENT_BINARY_DIR}/../clang-tidy")
add_clang_library(clangDaemon
   index/dex/Dex.cpp
   index/dex/Iterator.cpp
   index/dex/PostingList.cpp
   index/dex/Trigram.cpp
 
   refactor/Rename.cpp
   refactor/Tweak.cpp
 
-  LINK_LIBS
-  clangdSupport
-  clangTidy
-  ${LLVM_PTHREAD_LIB}
-  ${ALL_CLANG_TIDY_CHECKS}
-
   DEPENDS
   omp_gen
   )
 
 # Include generated CompletionModel headers.
 target_include_directories(clangDaemon PUBLIC
   $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>
 )
 
clang_target_link_libraries(clangDaemon
   clangSerialization
   clangTooling
   clangToolingCore
   clangToolingInclusions
   clangToolingRefactoring
   clangToolingSyntax
   )
 
+target_link_libraries(clangDaemon
+  PRIVATE
+  ${LLVM_PTHREAD_LIB}
+
+  clangTidy
+  ${ALL_CLANG_TIDY_CHECKS}
+
+  clangdSupport
+  clangdRemoteIndex
+  )
+
 add_subdirectory(refactor/tweaks)
 if (${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
   # FIXME: Make fuzzer not use linux-specific APIs, build it everywhere.
   add_subdirectory(fuzzer)
 endif()
 add_subdirectory(tool)
 add_subdirectory(indexer)
 

clang-tools-extra/clangd/index/remote/CMakeLists.txt

if (CLANGD_ENABLE_REMOTE)
 
   add_clang_library(clangdRemoteIndex
     Client.cpp
 
     LINK_LIBS
     RemoteIndexProto
     RemoteIndexServiceProto
     clangdRemoteMarshalling
-
     protobuf
     grpc++
-    clangDaemon
     clangdSupport
 
     DEPENDS
     RemoteIndexProto
     RemoteIndexServiceProto
     )
 
   add_subdirectory(marshalling)
   add_subdirectory(server)
 else()
   # Provides a dummy implementation of clangdRemoteIndex.
   add_subdirectory(unimplemented)
 endif()

clang-tools-extra/clangd/index/remote/Client.cpp

 //===--- Client.cpp ----------------------------------------------*- C++-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include <grpc++/grpc++.h>
 
 #include "Client.h"
 #include "Service.grpc.pb.h"
 #include "index/Index.h"
-#include "index/Serialization.h"
 #include "marshalling/Marshalling.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
 #include "clang/Basic/Version.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Error.h"
 
 #include <chrono>

clang-tools-extra/clangd/index/remote/unimplemented/CMakeLists.txt

 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../../../)
 # When compiled without Remote Index support, the real implementation index
 # client is not present. Users will get a notification about this when trying
 # to connect to remote index server instance.
 add_clang_library(clangdRemoteIndex
   UnimplementedClient.cpp
 
   LINK_LIBS
-  clangDaemon
   clangdSupport
   )

clang-tools-extra/clangd/tool/CMakeLists.txt

clang_target_link_libraries(clangd
   clangFormat
   clangFrontend
   clangLex
   clangSema
   clangTooling
   clangToolingCore
   clangToolingRefactoring
   clangToolingSyntax
-  clangdRemoteIndex
   )
+
 target_link_libraries(clangd
   PRIVATE
   clangTidy
+
   clangDaemon
+  clangdRemoteIndex
   clangdSupport
   ${CLANGD_XPC_LIBS}
   )