hokein added inline comments.
================ Comment at: clang-tools-extra/pseudo/CMakeLists.txt:1 +set(CLANG_PSEUDO_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}) + ---------------- sammccall wrote: > 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? removed ================ 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 ---------------- sammccall wrote: > it seems to me the build rules for files that end up in the include/ > directory should go here? sounds a good idea, move the cxx_gen.cmake to this file. ================ Comment at: clang-tools-extra/pseudo/lib/CMakeLists.txt:3 -add_clang_library(clangPseudo +# Needed by LLVM's CMake checks because this file defines multiple targets. +set(LLVM_OPTIONAL_SOURCES ---------------- sammccall wrote: > We do have a layering relationship here, and a requirement to keep the > "grammar" dependencies small - should we move it into a subdirectory? split it to a grammar subdirectory. 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