[PATCH] D60024: [cmake] Add function for building native tool

Shoaib Meenai via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 1 11:07:43 PDT 2019


smeenai marked an inline comment as done.
smeenai added inline comments.


================
Comment at: llvm/cmake/modules/CrossCompile.cmake:73
+  # LLVM_OPTIMIZED_TABLEGEN. Always builds in Release.
+  set(one_value_keywords
+    TARGET # The target to build natively
----------------
phosek wrote:
> Since both of these are required, wouldn't it be better to pass them as function arguments (i.e. `function(build_native_tool target output_path_var)`)?
Yeah, that's fair. I like the keyword form cos it's clearer at call sites, but not having a way to enforce required arguments (besides just manually verifying them) is annoying. Plus `DEPENDS` might go away in a follow-up, at which point it would be only function arguments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60024/new/

https://reviews.llvm.org/D60024





More information about the llvm-commits mailing list