[PATCH] D121763: [cmake] Provide CURRENT_TOOLS_DIR centrally, replacing CLANG_TOOLS_DIR
Haojian Wu via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 16 03:49:28 PDT 2022
hokein added a comment.
I think this patch is probably doing too much: 1. move CLANG_TOOLS_DIR to `configure_site_lit_cfg()` 2. rename.
Can we split it into two patches? Land the first part, and the second part afterwards.
================
Comment at: llvm/cmake/modules/AddLLVM.cmake:1640
+ # Like LLVM_{TOOLS,LIBS}_DIR, but pointing at the build tree.
+ string(REPLACE "${CMAKE_CFG_INTDIR}" "${LLVM_BUILD_MODE}" CURRENT_TOOLS_DIR "${LLVM_RUNTIME_OUTPUT_INTDIR}")
+ string(REPLACE "${CMAKE_CFG_INTDIR}" "${LLVM_BUILD_MODE}" CURRENT_LIBS_DIR "${LLVM_LIBRARY_OUTPUT_INTDIR}")
----------------
the change looks good, but I'm not sure the new name `CURRENT_TOOLS_DIR` is good, the name seems vague and not significantly better, since it always points at the build tree, maybe `LLVM_BUILD_DIR` or something similar? (The existing naming pattern looks like project-specific variables are prefixed with the project name `LLVM_`, `CLANG_`).
================
Comment at: llvm/utils/gn/secondary/clang-tools-extra/clangd/test/BUILD.gn:31
"CLANG_LIBS_DIR=", # needed only for shared builds
+ "CURRENT_TOOLS_DIR=",
----------------
nit: this seems not needed anymore, as we have removed the `CLANG_LIBS_DIR` from the CMake file already.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121763/new/
https://reviews.llvm.org/D121763
More information about the llvm-commits
mailing list