Dependencies between tools

Hi, I’m looking into integrating clang-tidy’s diagnostics into clangd, but I’m running into problems because clang-tidy’s headers seem effectively private from clangd’s point of view. (I can include “…/clang-tidy/ClangTidy.h”, but that seems like a pretty bad hack). Is there a way that the libraries in tools/extra can export public headers so that other tools can use them? Or should we be putting the functionality that we want to share in a library in clang proper?

Hi Chris,

Before getting into low-level details, could you elaborate on design you’re planning for this?
We had plans to integrate clang-tidy before, but it’s not as simple as running clang-tidy before reporting diagnostics.

Since clang-tidy diagnostics are slow, we want to make sure they don’t hurt user-experience of other features.
I.e. go-to-definition, compiler diags and code completion should not be blocked or get slower after we add clang-tidy.

Hi Ilya, I actually didn’t have much of a plan – I just wanted to see if I could do it. I was assuming that there would be a mechanism for starting asynchronous work that could add to the list of diagnostics, but I hadn’t gotten there yet. I do think that coming up with some sort of way to have inter-tool dependencies would be nice regardless of whether it’s currently feasible to add clang-tidy diagnostics to clangd. (I guess I’m saying that both problems will eventually need to be solved, and it makes sense to concentrate on them separately.)

I was assuming that there would be a mechanism for starting asynchronous work that could add to the list of diagnostics, but I hadn’t gotten there yet.

Your approach SG, just wanted to make sure we don’t block compiler diagnostics and other features while waiting on clang-tidy results.

We don’t have any support for background tasks in clangd, but we should definitely add them.
If you haven’t already, take a look at TUScheduler, it handles the threading in clangd and background tasks would probably go there.
Let us know if you have any questions along the way, there are certainly obstacles to making it work.

I do think that coming up with some sort of way to have inter-tool dependencies would be nice regardless of whether it’s currently feasible to add clang-tidy diagnostics to clangd. (I guess I’m saying that both problems will eventually need to be solved, and it makes sense to concentrate on them separately.)

Totally agree with you, sorry for drifting the conversation away from the problem at hand.
You’re right, all of clang-extra-tools are not structured as libraries. I guess the most principled approach would to follow LLVM’s convention and move public headers into include/, everything else to lib/, etc.

+Alexander Kornienko, have you considered extracting a public library interface for clang-tidy before? Any ideas on how to do it properly?

Clang-tidy has been used as a library since its appearance. But this was done outside the official source tree. If there’s interest in using it from clangd, but something is missing in the appropriate CMake configs, we should fix them. However, it looks like tools/extra/clang-tidy/CMakeLists.txt already defines add_clang_library(clangTidy ..., which should be enough?

What path should be used in the #include statement? I’m no CMake expert, but I interpreted add_clang_library only to affect the linking behavior, not what headers are visible.

I also don’t know much about CMake, but it looks like add_clang_library is enough for libraries in the Clang tree. Apparently, it does something specific to the clang tree layout (see macro(add_clang_library ... in cmake/modules/AddClang.cmake, it refers to ${CLANG_SOURCE_DIR}/include/clang/${lib_path}/*.h). Maybe a similar macro should be added for tools/extra to allow libraries to be exported in a similar way.

I guess the typical include path would be something like
"clang-tidy/whatever.h". For that to work, all you would need to do is to
add `include_directories(${CMAKE_CURRENT_SOURCE_DIR})` to the
CMakeLists.txt at the clang-tools-extra root.

Having the repository root in your include path is a bit weird, but there
isn't anything better we can do, given the current repo layout. For example
the other llvm projects (and other projects in general) put headers into a
special "include" folder (so e.g., llvm's Support/Path.h lives in
<repo_root>/include/llvm/Support/Path.h) and the include search path points
there. If you're willing to endure some (fairly invasive) layout changes
then we could rearrange the clang-tools-extra repo to follow that layout.
Otherwise, setting the include path to the repo root should be good enough.

I also don't know much about CMake, but it looks like add_clang_library

is enough for libraries in the Clang tree. Apparently, it does something
specific to the clang tree layout (see `macro(add_clang_library ...` in
cmake/modules/AddClang.cmake, it refers to
${CLANG_SOURCE_DIR}/include/clang/${lib_path}/*.h). Maybe a similar macro
should be added for tools/extra to allow libraries to be exported in a
similar way.

What path should be used in the #include statement? I'm no CMake

expert, but I interpreted add_clang_library only to affect the linking
behavior, not what headers are visible.

You're right, all of clang-extra-tools are not structured as

libraries. I guess the most principled approach would to follow LLVM's
convention and move public headers into include/, everything else to lib/,
etc.

+Alexander Kornienko, have you considered extracting a public library

interface for clang-tidy before? Any ideas on how to do it properly?

Clang-tidy has been used as a library since its appearance. But this

was done outside the official source tree. If there's interest in using it
from clangd, but something is missing in the appropriate CMake configs, we
should fix them. However, it looks like
tools/extra/clang-tidy/CMakeLists.txt already defines
`add_clang_library(clangTidy ...`, which should be enough?