[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