[PATCH] D120788: [cmake] Add INTERFACE_INCLUDE_DIRECTORIES to LLVM and MLIR.

[Stella Laurenzo via Phabricator via llvm-commits]

stellaraccident added inline comments.


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:501
+      FOLDER "Object Libraries"
+    )
     if(ARG_DEPENDS)
----------------
marbre wrote:
> With this, the interface include directories (usage-requirements) of the object libraries are mirrored. I would have assumed that mirroring the private include directories (build-requirements) might be sufficient, but seems I am wrong.
Ok, I think you may be right because of "The INTERFACE, PUBLIC and PRIVATE keywords are required to specify the scope of the following arguments. PRIVATE and PUBLIC items will populate the INCLUDE_DIRECTORIES property of <target>. PUBLIC and INTERFACE items will populate the INTERFACE_INCLUDE_DIRECTORIES property of <target>. The following arguments specify include directories."

So a `target_include_directories(Foo PUBLIC dir/)` will populate both the INCLUDE_DIRECTORIES and INTERFACE_INCLUDE_DIRECTORIES. By that logic, this should not be necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120788