[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

Petr Hosek via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 2 01:07:21 PST 2023


phosek added inline comments.


================
Comment at: cmake/Modules/GetClangResourceDir.cmake:17
+
+  set(oneValueArgs PREFIX SUBDIR)
+  cmake_parse_arguments(RESOURCE_DIR "" "${oneValueArgs}" "" ${ARGN})
----------------
This variable is only used once on the following line, I'd just inline it which is more idiomatic.


================
Comment at: cmake/Modules/GetClangResourceDir.cmake:18
+  set(oneValueArgs PREFIX SUBDIR)
+  cmake_parse_arguments(RESOURCE_DIR "" "${oneValueArgs}" "" ${ARGN})
+
----------------
It's more idiomatic to parse arguments at the start of the function.


================
Comment at: cmake/Modules/GetClangResourceDir.cmake:18
+  set(oneValueArgs PREFIX SUBDIR)
+  cmake_parse_arguments(RESOURCE_DIR "" "${oneValueArgs}" "" ${ARGN})
+
----------------
phosek wrote:
> It's more idiomatic to parse arguments at the start of the function.
This is just a nit, but we usually use a prefix like `ARGS` to make it clear that these are input arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141907



More information about the cfe-commits mailing list