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

[Andrzej Warzynski via Phabricator via flang-commits]

awarzynski added inline comments.


================
Comment at: flang/lib/FrontendTool/CMakeLists.txt:18
+
+if(TARGET clangBasic)
+  add_dependencies(flangFrontendTool clangBasic)
----------------
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?


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

https://reviews.llvm.org/D136606