[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR
Chris Bieneman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 13 08:17:20 PST 2023
beanz added inline comments.
================
Comment at: llvm/cmake/modules/AddLLVM.cmake:2401
+
+macro(setup_host_tool tool_name setting_name exe_var_name target_var_name)
+ cmake_parse_arguments(ARG "" "SCOPE" "" ${ARGN})
----------------
Please make this a `function` instead of a `macro`. In general CMake `macros` expand in ways that are unintuitive which can be a maintenance burden for people coming in and modifying the code.
Also a bit nity, but maybe a name like `find_host_program` would be more consistent naming. I suspect you could even simplify this implementation by using CMake's `find_program` (https://cmake.org/cmake/help/latest/command/find_program.html)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D131052/new/
https://reviews.llvm.org/D131052
More information about the cfe-commits
mailing list