[PATCH] D136651: [Clang] Give Clang the ability to use a shared stat cache

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D136651#4064474 <https://reviews.llvm.org/D136651#4064474>, @glandium wrote:

> In D136651#4064260 <https://reviews.llvm.org/D136651#4064260>, @glandium wrote:
>
>> This broke our mac builds with errors like:
>>
>>   ld64.lld: error: undefined symbol: CFRunLoopRun
>>   >>> referenced by tools/clang/tools/clang-stat-cache/CMakeFiles/clang-stat-cache.dir/clang-stat-cache.cpp.o:(symbol main+0x11be)
>>
>> (and many more symbols)
>
> Additional information: those are cross-compiled, and for some reason the `-framework CoreServices` in the CMakeLists.txt doesn't seem to make it to the command line, which is surprising because something similar works fine for e.g. dsymutil.

I ran into similar issues, not when cross compiling, but when compiling natively. In my case, I'm building with `-DLLVM_LINK_LLVM_DYLIB=ON`. Without that, it builds correctly for me.

The new tool needs to be linked with `-framework CoreServices`. This does get set in `clang/tools/clang-stat-cache/CMakeLists.txt` like this:

  if(APPLE)
  set(CLANG_STAT_CACHE_LIB_DEPS
    "-framework CoreServices"
    )
  endif()
  
  clang_target_link_libraries(clang-stat-cache
    PRIVATE
    ${CLANG_STAT_CACHE_LIB_DEPS}
    )

However, `clang_target_link_libraries` ignores extra dependencies when linking against the dylib: https://github.com/llvm/llvm-project/blob/a033dbbe5c43247b60869b008e67ed86ed230eaa/clang/cmake/modules/AddClang.cmake#L209-L213

  if (CLANG_LINK_CLANG_DYLIB)
    target_link_libraries(${target} ${type} clang-cpp)
  else()
    target_link_libraries(${target} ${type} ${ARGN})
  endif()

I guess `clang_target_link_libraries` needs a mechanism to disambiguate between generic clang library dependencies (which need to be dropped when linking against `clang-cpp` instead) and other dependencies which always are needed. I wonder if there's prior art for such disambiguation in other places - e.g. `llvm_add_library` does have similar logic for linking against either other llvm libraries or the dylib (and handles lots of other options). (I'm a bit out of time for digging further into this right now...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136651