[flang-commits] [PATCH] D136606: [flang] Fix building against clang dylib

[Michał Górny via Phabricator via flang-commits]

mgorny marked 2 inline comments as done.
mgorny added inline comments.


================
Comment at: flang/lib/FrontendTool/CMakeLists.txt:18
+
+if(TARGET clangBasic)
+  add_dependencies(flangFrontendTool clangBasic)
----------------
awarzynski wrote:
> mgorny wrote:
> > awarzynski wrote:
> > > Where is `TARGET` defined? Please bear with me - I'm confused that in Flang (which only defines Flang CMake targets) we would be checking whether "target" is a particular Clang target?
> > Unless I'm mistaken, if you're doing in-tree build of LLVM + Clang + Flang, then targets from other projects should be visible here. FWICS LLVM is ensuring that Clang is processed prior to Flang in all cases, so it should work.
> Sorry, I was not familiar with `if (TARGET <target>)`, hence my question. For future reference, it's documented under CMake's [[ https://cmake.org/cmake/help/latest/command/if.html#existence-checks | existance checks ]].
> 
> This makes sense, but I'm thinking that the following would be a bit more idiomatic. I'm also thinking that perhaps `libclang-cpp` should be added as a dependency too.
> ```
> if(CLANG_LINK_CLANG_DYLIB)
>  add_dependencies(flangFrontendTool clangBasic)
> else()
>   add_dependencies(flangFrontendTool clang-cpp)
> endif()
> ```
> 
> `CLANG_LINK_CLANG_DYLIB` [[ https://github.com/llvm/llvm-project/blob/d5953785a45595345c567440bd19aa379d59fe8e/clang/CMakeLists.txt#L281-L282 | is defined ]] whenever `LLVM_LINK_LLVM_DYLIB` is. So that ^^^ should be safe and always correct. WDYT?
I suppose you meant `if` the other way around but yes, it should work. I'm going to try it right now and update the patch in a minute.


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

https://reviews.llvm.org/D136606