kadircet added a comment. Mostly a review of the clangd changes, I am not familiar with all the other parts (and not necessarily everyone will be) hence some explicit testing results would be great. Also please upload the patch with full context.
================ Comment at: clang-tools-extra/clangd/CMakeLists.txt:41 - Support - AllTargetsInfos - FrontendOpenMP ---------------- clangd actually requires `AllTargetsInfos` so that it can display detailed semantic information like bitwidth of certain types. not just on the final binary (as I see you've included this on the tool/CMakeLists.txt), there are also tests that depend on it. ================ Comment at: clang-tools-extra/clangd/indexer/CMakeLists.txt:11 PRIVATE - clangAST clangBasic ---------------- these are all required dependencies of clangd-indexer. i suppose you're getting rid of these as they're transitively linked in by clangDeamon, but I remember some build configurations (shared lib builds) still requiring these to be present at these levels as well. i might be wrong, but it would be better to show some explicit testing results claiming these are not causing breakages (as Mehdi pointed out). ================ Comment at: clang-tools-extra/clangd/refactor/tweaks/CMakeLists.txt:35 clangdSupport - clangFormat clangLex ---------------- this might require extra cleanups on the headers of some sources. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120375/new/ https://reviews.llvm.org/D120375 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
