Page MenuHomePhabricator

[lldb] [cmake] Support linking against clang-cpp dylib
ClosedPublic

Authored by mgorny on Oct 4 2019, 4:13 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

mgorny created this revision.Oct 4 2019, 4:13 AM

FTR, I'm getting missing LD_LIBRARY_PATH problems when running check-llvm with dylibs. However, it also happens with plain LLVM_LINK_LLVM_DYLIB, so I don't think it needs to be addressed with this patch.

labath added a comment.Oct 4 2019, 4:19 AM

Seems reasonable. Just one quick question about the implementation.

lldb/cmake/modules/AddLLDB.cmake
94–99 ↗(On Diff #223185)

I don't know whether this makes any difference, but I am wondering if we should pass these (both clang-cpp and PARAM_CLANG_LIBS) as the LINK_LIBS argument to the llvm_add_library call above. Seems like it would be more consistent with the approach of letting the llvm function manage linking. WDYT?

mgorny marked an inline comment as done.Oct 4 2019, 4:23 AM
mgorny added inline comments.
lldb/cmake/modules/AddLLDB.cmake
94–99 ↗(On Diff #223185)

I presumed it doesn't, so went for the simpler approach ;-). I can change it if you prefer.

labath accepted this revision.Oct 4 2019, 4:33 AM
labath added inline comments.
lldb/cmake/modules/AddLLDB.cmake
94–99 ↗(On Diff #223185)

Pfft.. I don't know.. We can leave that as is..

This revision is now accepted and ready to land.Oct 4 2019, 4:33 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 4 2019, 5:05 AM

What about this patch: it's much more simple, though maybe I missed something: https://reviews.llvm.org/D68071