[PATCH] D155540: [clangd] Remove extra dependancies for clangd

2023-08-11 Thread Lei Huang via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe6f18b75d7ff: [clangd] Remove extra dependancies for clangd (authored by saghir, committed by lei). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D155540: [clangd] Remove extra dependancies for clangd

2023-08-11 Thread Lei Huang via Phabricator via cfe-commits
lei updated this revision to Diff 549374. lei added a comment. Rebase to ToT Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155540/new/ https://reviews.llvm.org/D155540 Files: clang-tools-extra/clangd/tool/CMakeLists.txt Index:

[PATCH] D155540: [clangd] Remove extra dependancies for clangd

2023-08-09 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D155540#4574613 , @lei wrote: > I was thinking to commit this patch to minimize the dependencies if there are > no objections. @MaskRay @nemanjai any preferences? LGTM. Please commit it. Repository: rG LLVM Github

[PATCH] D155540: [clangd] Remove extra dependancies for clangd

2023-08-09 Thread Lei Huang via Phabricator via cfe-commits
lei added a comment. I was thinking to commit this patch to minimize the dependencies if there are no objections. @MaskRay @nemanjai any preferences? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155540/new/ https://reviews.llvm.org/D155540

[PATCH] D155540: [clangd] Remove extra dependancies for clangd

2023-07-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. In D155540#4524326 , @sammccall wrote: > In D155540#4524219 , @MaskRay wrote: > >> `llvm/cmake/modules/HandleLLVMOptions.cmake` passes `-Wl,-z,defs`. >> `-DBUILD_SHARED_LIBS=on` builds

[PATCH] D155540: [clangd] Remove extra dependancies for clangd

2023-07-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment. In D155540#4524219 , @MaskRay wrote: > `llvm/cmake/modules/HandleLLVMOptions.cmake` passes `-Wl,-z,defs`. > `-DBUILD_SHARED_LIBS=on` builds get checking from the linker option >

[PATCH] D155540: [clangd] Remove extra dependancies for clangd

2023-07-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. `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. For a

[PATCH] D155540: [clangd] Remove extra dependancies for clangd

2023-07-21 Thread Ahsan Saghir via Phabricator via cfe-commits
saghir added a comment. Thank you for the review and your comments @sammccall and @mstorsjo. As I understand there is no strong opinion here either way. I think we can leave things as they are for now. I am going to abandon this patch. If someone else feels strongly about removing them, they

[PATCH] D155540: [clangd] Remove extra dependancies for clangd

2023-07-21 Thread Sam McCall via Phabricator via cfe-commits
sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. I don't have a strong opinion here (or solid understanding of dynamic linking!) I suppose the idea is that clangdMain has dependencies from the source files in this directory, and the

[PATCH] D155540: [clangd] Remove extra dependancies for clangd

2023-07-18 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a reviewer: sammccall. mstorsjo added a subscriber: sammccall. mstorsjo added a comment. Thanks for looking at my suggestion! At this point, I'd defer the judgement to the code owner, @sammccall - whether it's nicer to have a tighter set of dependencies, or to reduce the risk of