compnerd added inline comments.
================
Comment at: clang/cmake/modules/AddClang.cmake:127
+ LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}
+ ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}${LLVM_LIBDIR_SUFFIX}
+ RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR})
----------------
For the initial change, Id leave this off. `CMAKE_INSTALL_LIBDIR` sometimes
already deals with the bit suffix, so you can end up with two instances of `32`
or `64`. I think that cleaning that up separately, while focusing on the
details of cleaning up how to handle `LLVM_LIBDIR_SUFFIX` is the right thing to
do. The same applies to the rest of the patch.
================
Comment at: compiler-rt/cmake/base-config-ix.cmake:72
+ set(COMPILER_RT_INSTALL_PATH "" CACHE PATH
+ "Prefix where built compiler-rt artifacts should be installed, comes
before CMAKE_INSTALL_PREFIX.")
option(COMPILER_RT_INCLUDE_TESTS "Generate and build compiler-rt unit
tests." OFF)
----------------
Please don't change the descriptions of the options as part of the
`GNUInstallDirs` handling. The change to `COMPILER_RT_INSTALL_PATH` looks
incorrect. Can you explain this change please?
================
Comment at: libcxx/CMakeLists.txt:32
+
+include(GNUInstallDirs)
----------------
Does this need to come here? Why not push this to after the if block
completes? The same applies through out the rest of the patch.
================
Comment at: libcxx/cmake/Modules/HandleLibCXXABI.cmake:66
install(FILES "${LIBCXX_BINARY_INCLUDE_DIR}/${fpath}"
- DESTINATION ${LIBCXX_INSTALL_HEADER_PREFIX}include/c++/v1/${dstdir}
+ DESTINATION
${LIBCXX_INSTALL_HEADER_PREFIX}${CMAKE_INSTALL_INCLUDEDIR}/c++/v1/${dstdir}
COMPONENT cxx-headers
----------------
@ldionne - how is the `LIBCXX_INSTALL_HEADER_PREFIX` used? Can altering the
value of `CMAKE_INSTALL_INCLUDEDIR` serve the purpose?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99484/new/
https://reviews.llvm.org/D99484
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
