[PATCH] D122270: Support converting pointers from opaque to typed

Jakub Kuderski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun May 8 19:31:43 PDT 2022


kuhar added inline comments.


================
Comment at: llvm/lib/Target/DirectX/DXILWriter/CMakeLists.txt:1
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../)
+
----------------
beanz wrote:
> kuhar wrote:
> > Could we use `target_include_directories` instead or a global ones?
> `include_directories` sets the directory property not the global one, so it only applies to this directory and any subdirectories. There's no reason we couldn't use the target-specific one, but there is no material difference.
I see this similar to using global variables in C++. There's no difference *now* but in my experience cmake tends to grow by copy-paste-style development and in X years we may have N targets here all with the same include directories. As we port these to other builds systems like GN or blaze, having clear `target_` includes/libraries/definitions makes life much easier.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122270



More information about the llvm-commits mailing list