[clangd] Add library for clangd main function

[ivanmurashko]

Summary

The diff adds a library and header for clangd main function. That change allows to create custom builds for clangd outside the main LLVM repo. The diff also allows to use build system different from CMake to build clangd. The main reason for such change is an ability to use custom clang-tidy modules (created outside LLVM repo).

Test Plan:

ninja clangd

also check that necessary libs are installed aka

ninja install
...
ls <install folder>/lib/libclangdMain.a

[sammccall]

IIUC we want to link some extra targets into the clangd binary in order to provide extra clang-tidy checks, but *don't* want to customize the binary further.
(The latter can be useful, but this interface isn't sufficient for it).

Can this be solved at the build-system level, without changing the source code in ways that aren't otherwise useful?
For example, with a CLANG_TIDY_EXTRA_CHECKS cmake variable that adds more deps? This seems like it could also work for the clang-tidy binary.

[ivanmurashko]

The change addresses comments from D145228. The only one header is copied as a part of the installation. As result the diff contains the minimal changes required to integrated clangd into custom build systems (different from CMake).

[ivanmurashko]

Can this be solved at the build-system level, without changing the source code in ways that aren't otherwise useful?
For example, with a CLANG_TIDY_EXTRA_CHECKS cmake variable that adds more deps? This seems like it could also work for the clang-tidy binary.

As it was mentioned at D145228, we use LLVM as a set of libraries and headers that allow us to create custom components. The build system we use for the component is different from CMake (it's buck). Thus, unfortunately, the CMake customization does not work for us.

IIUC we want to link some extra targets into the clangd binary in order to provide extra clang-tidy checks, but *don't* want to customize the binary further.
(The latter can be useful, but this interface isn't sufficient for it).

I agree that the current interface does not provide any way to customize the binary. My primary reason was the following: I tried to keep the changes as minimal as possible to get an ability to build the clangd outside LLVM repo. I assumed that additional customization API can be added later as soon as it is be required.

BTW: I added the copy of the single header file (API) instead of copying all internal headers. That will allow to abandon D145228 and add future external API at the select set of headers in future.

[ivanmurashko]

rebased to the latest LLVM main

[ivanmurashko]

removed header install, only lib install has been left after the change

CC: @kadircet

[kadircet]

thanks LG and seems to be working in a couple build configurations I tried. but there might still be breakages in different configs, so please be on the watchout after landing this for breakages in https://lab.llvm.org/.

FWIW something like:

# Needed by LLVM's CMake checks because this file defines multiple targets.
set(LLVM_OPTIONAL_SOURCES ClangdToolMain.cpp)

add_clang_library(clangdMain
  Check.cpp
  ClangdMain.cpp
  )

clang_target_link_libraries(clangdMain
  PRIVATE
  clangAST
  clangBasic
  clangDaemon
  clangFormat
  clangFrontend
  clangLex
  clangSema
  clangTidy
  clangTooling
  clangToolingCore
  clangToolingRefactoring
  clangToolingSyntax
  clangdRemoteIndex
  clangdSupport
  $<TARGET_OBJECTS:obj.clangDaemonTweaks>
)

if(CLANGD_BUILD_XPC)
  target_link_libraries(clangdMain
    PRIVATE
    clangdXpcJsonConversions
    clangdXpcTransport
    )
endif()

add_clang_tool(clangd
  ClangdToolMain.cpp
  )

clang_target_link_libraries(clangd
  PRIVATE
  clangdMain
  )

for the CMakeFile seem to cut it.

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

11

we should move this into clangdMain target now

25

it should be enough to have clangdMain here now.

34–35

you can get rid of this rule too

37

you can merge this with the previous rule

[ivanmurashko]

Rebase + apply suggestions from @kadircet

[ivanmurashko]

small fixes

[ivanmurashko]

Fixed clangd unit tests

[ivanmurashko]

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

11

Unfortunately it did not work and broke the clangd unit tests.

Note: all other comments were fixed

[ivanmurashko]

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

34–35

fixed

37

fixed

[ivanmurashko]

In D145302#4441334, @kadircet wrote:

thanks LG and seems to be working in a couple build configurations I tried. but there might still be breakages in different configs, so please be on the watchout after landing this for breakages in https://lab.llvm.org/.

@kadircet , could you look at the final version?

[ivanmurashko]

rebase before the final push

[kadircet]

thanks

[glandium]

This broke building with -DLLVM_LINK_LLVM_DYLIB=ON:

/usr/bin/ld: tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/AddUsing.cpp.o: in function `clang::clangd::(anonymous namespace)::AddUsing::prepare(clang::clangd::Tweak::Selection const&)':
AddUsing.cpp:(.text._ZN5clang6clangd12_GLOBAL__N_18AddUsing7prepareERKNS0_5Tweak9SelectionE+0x1c): undefined reference to `clang::clangd::ParsedAST::getASTContext()'
/usr/bin/ld: AddUsing.cpp:(.text._ZN5clang6clangd12_GLOBAL__N_18AddUsing7prepareERKNS0_5Tweak9SelectionE+0x6d): undefined reference to `clang::clangd::ParsedAST::getASTContext() const'
/usr/bin/ld: AddUsing.cpp:(.text._ZN5clang6clangd12_GLOBAL__N_18AddUsing7prepareERKNS0_5Tweak9SelectionE+0x9a): undefined reference to `clang::clangd::isHeaderFile(llvm::StringRef, std::optional<clang::LangOptions>)'
/usr/bin/ld: AddUsing.cpp:(.text._ZN5clang6clangd12_GLOBAL__N_18AddUsing7prepareERKNS0_5Tweak9SelectionE+0xcc): undefined reference to `clang::clangd::SelectionTree::commonAncestor() const'
/usr/bin/ld: AddUsing.cpp:(.text._ZN5clang6clangd12_GLOBAL__N_18AddUsing7prepareERKNS0_5Tweak9SelectionE+0x391): undefined reference to `clang::clangd::printNamespaceScope[abi:cxx11](clang::DeclContext const&)'

etc.

[raj.khem]

indeed, broke my CI with same issue.

In D145302#4491973, @glandium wrote:

This broke building with -DLLVM_LINK_LLVM_DYLIB=ON:

/usr/bin/ld: tools/clang/tools/extra/clangd/refactor/tweaks/CMakeFiles/obj.clangDaemonTweaks.dir/AddUsing.cpp.o: in function `clang::clangd::(anonymous namespace)::AddUsing::prepare(clang::clangd::Tweak::Selection const&)':
AddUsing.cpp:(.text._ZN5clang6clangd12_GLOBAL__N_18AddUsing7prepareERKNS0_5Tweak9SelectionE+0x1c): undefined reference to `clang::clangd::ParsedAST::getASTContext()'
/usr/bin/ld: AddUsing.cpp:(.text._ZN5clang6clangd12_GLOBAL__N_18AddUsing7prepareERKNS0_5Tweak9SelectionE+0x6d): undefined reference to `clang::clangd::ParsedAST::getASTContext() const'
/usr/bin/ld: AddUsing.cpp:(.text._ZN5clang6clangd12_GLOBAL__N_18AddUsing7prepareERKNS0_5Tweak9SelectionE+0x9a): undefined reference to `clang::clangd::isHeaderFile(llvm::StringRef, std::optional<clang::LangOptions>)'
/usr/bin/ld: AddUsing.cpp:(.text._ZN5clang6clangd12_GLOBAL__N_18AddUsing7prepareERKNS0_5Tweak9SelectionE+0xcc): undefined reference to `clang::clangd::SelectionTree::commonAncestor() const'
/usr/bin/ld: AddUsing.cpp:(.text._ZN5clang6clangd12_GLOBAL__N_18AddUsing7prepareERKNS0_5Tweak9SelectionE+0x391): undefined reference to `clang::clangd::printNamespaceScope[abi:cxx11](clang::DeclContext const&)'

etc.

[mstorsjo]

In D145302#4491973, @glandium wrote:

This broke building with -DLLVM_LINK_LLVM_DYLIB=ON:

I also ran into this; I pushed a fix in a20d57e83441a69fa2bab86593b18cc0402095d2.

[ivanmurashko]

In D145302#4492595, @mstorsjo wrote:

In D145302#4491973, @glandium wrote:

This broke building with -DLLVM_LINK_LLVM_DYLIB=ON:

I also ran into this; I pushed a fix in a20d57e83441a69fa2bab86593b18cc0402095d2.

Thanks for the fix!

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

-add_clang_tool(clangd
+# Needed by LLVM's CMake checks because this file defines multiple targets.
+set(LLVM_OPTIONAL_SOURCES ClangdToolMain.cpp)
+
+add_clang_library(clangdMain
   ClangdMain.cpp
   Check.cpp
+  )
+
+add_clang_tool(clangd
+  ClangdToolMain.cpp
   $<TARGET_OBJECTS:obj.clangDaemonTweaks>
   )
 
 set(LLVM_LINK_COMPONENTS
   support
   )
 
 set(CLANGD_XPC_LIBS "")
 if(CLANGD_BUILD_XPC)
   list(APPEND CLANGD_XPC_LIBS "clangdXpcJsonConversions" "clangdXpcTransport")
 endif()
 
-clang_target_link_libraries(clangd
+clang_target_link_libraries(clangdMain
   PRIVATE
   clangAST
   clangBasic
   clangFormat
   clangFrontend
   clangLex
   clangSema
   clangTooling
   clangToolingCore
   clangToolingRefactoring
   clangToolingSyntax
-  )
-
-target_link_libraries(clangd
-  PRIVATE
   clangTidy
-
   clangDaemon
   clangdRemoteIndex
   clangdSupport
   ${CLANGD_XPC_LIBS}
   )
+
+clang_target_link_libraries(clangd
+  PRIVATE
+  clangdMain
+  )

clang-tools-extra/clangd/tool/ClangdMain.h

+//===--- ClangdMain.h - clangd main function ------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TOOL_CLANGDMAIN_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TOOL_CLANGDMAIN_H
+
+namespace clang {
+namespace clangd {
+// clangd main function (clangd server loop)
+int clangdMain(int argc, char *argv[]);
+} // namespace clangd
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_TOOL_CLANGDMAIN_H

clang-tools-extra/clangd/tool/ClangdMain.cpp

 //===--- ClangdMain.cpp - clangd server loop ------------------------------===//
 //
 // 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 "ClangdMain.h"
 #include "ClangdLSPServer.h"
 #include "CodeComplete.h"
 #include "Compiler.h"
 #include "Config.h"
 #include "ConfigProvider.h"
 #include "Feature.h"
 #include "IncludeCleaner.h"
 #include "PathMapping.h"
Frag = [=](const config::Params &, Config &C) {
 
       if (Test)
         C.Index.StandardLibrary = false;
       return true;
     };
   }
 };
 } // namespace
-} // namespace clangd
-} // namespace clang
 
 enum class ErrorResultCode : int {
   NoShutdownRequest = 1,
   CantRunAsXPCService = 2,
   CheckFailed = 3
 };
 
-int main(int argc, char *argv[]) {
-  using namespace clang;
-  using namespace clang::clangd;
-
+int clangdMain(int argc, char *argv[]) {
   llvm::InitializeAllTargetInfos();
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   llvm::sys::AddSignalHandler(
       [](void *) {
         ThreadCrashReporter::runCrashHandlers();
         // Ensure ThreadCrashReporter and PrintStackTrace output is visible.
         llvm::errs().flush();
       },
#endif
   // These should terminate quickly, and ~ClangdLSPServer blocks on them.
   // However if a bug causes them to run forever, we want to ensure the process
   // eventually exits. As clangd isn't directly user-facing, an editor can
   // "leak" clangd processes. Crashing in this case contains the damage.
   abortAfterTimeout(std::chrono::minutes(5));
 
   return ExitCode;
 }
+
+} // namespace clangd
+} // namespace clang

clang-tools-extra/clangd/tool/ClangdToolMain.cpp

+//===--- ClangdToolMain.cpp - clangd main function ------------------------===//
+//
+// 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 "ClangdMain.h"
+
+int main(int argc, char **argv) {
+  return clang::clangd::clangdMain(argc, argv);
+}