[PATCH] D52710: [clangd] Add "check-clangd" target

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 1 03:01:35 PDT 2018


sammccall added inline comments.


================
Comment at: test/CMakeLists.txt:76
 
-set(llvm_utils
-  FileCheck count not
-  )
-
-foreach(t ${llvm_utils})
-  if(TARGET ${t})
-    list(APPEND CLANG_TOOLS_TEST_DEPS ${t})
-  endif()
-endforeach()
-
+macro(add_llvm_utils_deps deps)
+  set(llvm_utils
----------------
hokein wrote:
> sammccall wrote:
> > Conversely, I'd consider just expanding these out into each of the dep lists rather than keeping two levels of indirection here.
> > 
> > 
> Ah, I missed this comment. It seems that there is no elegant way to expand these to `CLANG_TOOLS_TEST_DEPS` and `CLANGD_TOOLS_TEST_DEPS`, we have to repeat the code twice. So I'd prefer using macro to eliminate code duplication.
Right, that was my suggestion: instead of 13 lines of macros, to add "FileCheck count not" twice, we just write it twice. It won't change often, and if it does there's no reason to assume you'd want them in sync.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52710





More information about the cfe-commits mailing list