mgorny marked 2 inline comments as done. mgorny added inline comments.
================ Comment at: clang-tools-extra/clangd/CMakeLists.txt:105 + +clang_target_link_libraries(clangDaemon + PRIVATE ---------------- sammccall wrote: > This has split our link dependencies in two, and I'm not really sure why (and > so am likely to put future dependencies in the wrong place). > > Can *all* the link dependencies be moved to the clang_target_link_libraries > section? > ((Even if this addresses a problem only seen with clang libs, I'd rather have > everything in one place) No. `clang_target_link_libraries` is only for libraries that are included in `libclang-cpp.so`, so when the latter is built, they are replaced with it entirely. I'm all for improving this (e.g. to automatically replace only these libs that are included in clang-cpp) but I don't really have time to work on this beyond fixing immediate failures. ================ Comment at: clang-tools-extra/clangd/unittests/CMakeLists.txt:123 clangTidy - LLVMSupport LLVMTestingSupport ---------------- sammccall wrote: > why this change? We do depend directly on LLVMSupport, and I'd prefer that to > remain explicit rather than pick it up transitively. It's already listed in `LLVM_LINK_COMPONENTS` which handles using libLLVM correctly. Listing it here results in another copy of LLVMSupport being linked in which caused the test to crash immediately on duplicate command-line options. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81967/new/ https://reviews.llvm.org/D81967 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
