beanz requested changes to this revision. beanz added inline comments. This revision now requires changes to proceed.
================ Comment at: clang/tools/libclang/CMakeLists.txt:115 +clang_target_link_libraries(libclang + PRIVATE + ${CLANG_LIB_DEPS} ---------------- tstellar wrote: > aaronpuchert wrote: > > This might not be correct for static builds, I think we need `INTERFACE` > > here. > This patch looks OK to me, but you should find someone with more CMake > knowledge to answer this question. This part of the patch is a bit tricky. As implemented it is fine for the most common build configurations, but will be a problem if `LIBCLANG_BUILD_STATIC=On` or if `LLVM_ENABLE_PIC=Off`. The correct solution is probably to wrap this code in `if (ENABLE_SHARED)`, and to have another code block that handles `if (ENABLE_STATIC)`. In that block you need to call this with `INTERFACE` as the linkage type, and you'll need to handle the case where both `ENABLE_SHARED` and `ENABLE_STATIC` is set. In that case the static library target is named `libclang_static`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67321/new/ https://reviews.llvm.org/D67321 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits