[PATCH] D155540: [clangd] Remove extra dependancies for clangd
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jul 21 16:54:51 PDT 2023
sammccall added a comment.
In D155540#4524219 <https://reviews.llvm.org/D155540#4524219>, @MaskRay wrote:
> `llvm/cmake/modules/HandleLLVMOptions.cmake` passes `-Wl,-z,defs`. `-DBUILD_SHARED_LIBS=on` builds get checking from the linker option (https://maskray.me/blog/2021-06-13-dependency-related-linker-options#z-defs). This is similar Bazel's layering_check.
Thanks! This is good to know - it seems like this means when things break I can repro locally and should get the same results, if I anticipate a breakage I can test for it. I can't easily tell if I have *too many* deps, but that's probably not an urgent problem.
> For a dependency, say, `clangTooling`, if a source file within the `clangd` target (executable) uses (directly or transitively) clangTooling, the dependency should be kept, otherwise it should be removed.
> If the source files are completely IWYU clean, it should be straightforward to tell whether a dependency can be removed by inspecting the `#include` lines.
I'm not sure this is the case:
// clangdMain.cpp
#include "clangDaemon.h"
int result = clangDaemonThings();
// clangDaemon.h
#include "Tooling.h"
inline int clangDaemonThings() { tooling::doStuff(); }
IIUC the call to `clangDaemonThings` gets inlined, now `clangdMain` has to be linked against `Tooling`, despite being IWYU-clean and not including any of its headers.
(This would be unlike bazel, which doesn't require `cc_library(name="clangdMain", deps=["clangTooling"])` in that case, maybe I'm misunderstanding though...)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155540/new/
https://reviews.llvm.org/D155540
More information about the cfe-commits
mailing list