[PATCH] D130553: [clang][lld][cmake] Simplify header dirs

[Sebastian Neubauer via Phabricator via cfe-commits]

sebastian-ne accepted this revision.
sebastian-ne added a comment.
This revision is now accepted and ready to land.

Looks good to me, I left three questions inline.



================
Comment at: clang/CMakeLists.txt:81
     # path is removed.
-    set(MAIN_INCLUDE_DIR "${LLVM_INCLUDE_DIR}")
+    set(INCLUDE_DIRS ${LLVM_INCLUDE_DIRS})
     set(LLVM_OBJ_DIR "${LLVM_BINARY_DIR}")
----------------
Ericson2314 wrote:
> sebastian-ne wrote:
> > Do we need to add `"${LLVM_BINARY_DIR}/include"` here as well?
> Pretty sure we shouldn't be cause that should be one of the elements of the list when LLVM is also being built in this CMake invocation.
The quotation marks went missing, is this intended? (Same in lld/CMakeLists.txt)


================
Comment at: clang/CMakeLists.txt:91
 
-  set(LLVM_MAIN_INCLUDE_DIR "${MAIN_INCLUDE_DIR}" CACHE PATH "Path to llvm/include")
+  set(LLVM_INCLUDE_DIRS ${INCLUDE_DIRS} CACHE PATH "Path to llvm/include and any other header dirs needed")
   set(LLVM_BINARY_DIR "${LLVM_OBJ_ROOT}" CACHE PATH "Path to LLVM build tree")
----------------
The quotation marks went missing, is this intended? (Same in lld/CMakeLists.txt)


================
Comment at: clang/CMakeLists.txt:133
 
-  include_directories("${LLVM_BINARY_DIR}/include" "${LLVM_MAIN_INCLUDE_DIR}")
+  include_directories(${LLVM_INCLUDE_DIRS})
   link_directories("${LLVM_LIBRARY_DIR}")
----------------
The quotation marks went missing, is this intended? (Same in lld/CMakeLists.txt)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130553