wallace created this revision.
wallace added a reviewer: rriddle.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
If we want to export all lldb symbols (i.e
LLDB_EXPORT_ALL_SYMBOLS=ON), we need to use de
wallace updated this revision to Diff 510550.
wallace added a comment.
nits
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147453/new/
https://reviews.llvm.org/D147453
Files:
lldb/cmake/modules/AddLLDB.cmake
Index: lldb/cmake/modules/AddLLDB.cm
wallace updated this revision to Diff 510551.
wallace added a comment.
another nit...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147453/new/
https://reviews.llvm.org/D147453
Files:
lldb/cmake/modules/AddLLDB.cmake
Index: lldb/cmake/modules/
rriddle added inline comments.
Comment at: lldb/cmake/modules/AddLLDB.cmake:173
+ if (LLDB_EXPORT_ALL_SYMBOLS)
+if (CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU")
+ target_compile_options(${name} PRIVATE "-fvisibility=default")
Other places uses `if (NOT CM
wallace added inline comments.
Comment at: lldb/cmake/modules/AddLLDB.cmake:173
+ if (LLDB_EXPORT_ALL_SYMBOLS)
+if (CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU")
+ target_compile_options(${name} PRIVATE "-fvisibility=default")
rriddle wrote:
> Other places
wallace updated this revision to Diff 510557.
wallace added a comment.
gate the target OS
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147453/new/
https://reviews.llvm.org/D147453
Files:
lldb/cmake/modules/AddLLDB.cmake
Index: lldb/cmake/modu
rriddle accepted this revision.
rriddle added a comment.
This revision is now accepted and ready to land.
LGTM, and makes sense to me given that downstream users often want a build of
LLVM with `CMAKE_CXX_VISIBILITY_PRESET=hidden`, but lldb should still be able
to build/link/work in the presence
JDevlieghere added inline comments.
Comment at: lldb/cmake/modules/AddLLDB.cmake:175
+CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU")
+ target_compile_options(${name} PRIVATE "-fvisibility=default")
+endif()
Rather than changing the compile options di
wallace added inline comments.
Comment at: lldb/cmake/modules/AddLLDB.cmake:175
+CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU")
+ target_compile_options(${name} PRIVATE "-fvisibility=default")
+endif()
JDevlieghere wrote:
> Rather than changing the c
wallace updated this revision to Diff 510588.
wallace added a comment.
Address comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147453/new/
https://reviews.llvm.org/D147453
Files:
lldb/cmake/modules/AddLLDB.cmake
Index: lldb/cmake/module
JDevlieghere accepted this revision.
JDevlieghere added a comment.
Thanks! LGTM
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147453/new/
https://reviews.llvm.org/D147453
___
lldb-commits mailing list
ll
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe538c6fc3048: [LLDB] Ensure LLDB symbols are exported in
LLDB_EXPORT_ALL_SYMBOLS is provided. (authored by wallace).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm
12 matches
Mail list logo