sammccall added inline comments.

================
Comment at: clang-tools-extra/pseudo/CMakeLists.txt:1
+set(CLANG_PSEUDO_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
----------------
I think these variables shared across CMakeLists.txt files generally add more 
confusion than value, it doesn't seem to be needed here - can we use relative 
paths instead?


================
Comment at: clang-tools-extra/pseudo/gen/cxx_gen.cmake:1
+# The cxx.bnf grammar file
+set(cxx_bnf ${CMAKE_CURRENT_SOURCE_DIR}/../lib/cxx.bnf)
----------------
why is this a textually included *.cmake file that adds rules, instead of a 
CMakeLists.txt file or a *.cmake file that provides functions?


================
Comment at: clang-tools-extra/pseudo/include/CMakeLists.txt:1
+# We put an empty cmake file here so that cmake can create an include directory
+# in the build directory, the include directory is the home for generated 
source
----------------
it seems to me the build rules for files that end up in the include/ directory 
should go here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125667

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to