[PATCH] D58537: lib/Header: Simplify CMakeLists.txt

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai accepted this revision.
smeenai added reviewers: beanz, phosek.
smeenai added a comment.
This revision is now accepted and ready to land.

LGTM, though I generally prefer functions to macros.



================
Comment at: clang/lib/Headers/CMakeLists.txt:127
 
-# Generate arm_neon.h
-clang_tablegen(arm_neon.h -gen-arm-neon
-  -I ${CLANG_SOURCE_DIR}/include/clang/Basic/
-  SOURCE ${CLANG_SOURCE_DIR}/include/clang/Basic/arm_neon.td)
-# Generate arm_fp16.h
-clang_tablegen(arm_fp16.h -gen-arm-fp16
-  -I ${CLANG_SOURCE_DIR}/include/clang/Basic/
-  SOURCE ${CLANG_SOURCE_DIR}/include/clang/Basic/arm_fp16.td)
-
-set(out_files)
-foreach( f ${files} ${cuda_wrapper_files} )
-  set( src ${CMAKE_CURRENT_SOURCE_DIR}/${f} )
-  set( dst ${output_dir}/${f} )
+macro(copy_header_to_output_dir src_dir file)
+  set(src ${src_dir}/${file})
----------------
I'd prefer this be converted to a function and the append to  `out_files` be handled explicitly via `PARENT_SCOPE` (you could either hardcode the name or take in a parameter for the name of the list to append to). Macros pollute variables and their parameters behave weirdly.


================
Comment at: clang/lib/Headers/CMakeLists.txt:137
+
+macro(clang_generate_header td_option td_file out_file)
+  clang_tablegen(${out_file} ${td_option}
----------------
This one can trivially be a function instead of a macro, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58537