[libcxx-commits] [PATCH] D60166: [libc++abi] Refactor CMake build to avoid object libraries

Tue Apr 2 17:45:50 PDT 2019

smeenai added inline comments.

================
Comment at: libcxxabi/src/CMakeLists.txt:206
+    if(LIBUNWIND_HERMETIC_STATIC_LIBRARY)
+      target_sources(cxxabi_static PRIVATE $<TARGET_OBJECTS:unwind_static_objects>)
+    else()
----------------
smeenai wrote:
> smeenai wrote:
> > ldionne wrote:
> > > smeenai wrote:
> > > > Will you want to do something similar with libunwind (where it's not build as an object library anymore), at which point this logic will change too?
> > > If we don't build object libraries for libunwind, then this would change to `$<TARGET_OBJECTS:unwind_static>` and `$<TARGET_OBJECTS:unwind>` respectively, IIUC.
> > `$<TARGET_OBJECTS:...>` only works with object libraries (https://cmake.org/cmake/help/v3.4/manual/cmake-generator-expressions.7.html), so it'd need a bit more tweaking, but yeah, that's kinda independent of this patch :)
> > 
> > I did realize a problem with this though. In our minimum supported CMake version (3.4.3), you can only use TARGET_SOURCES inside the sources list of `add_library` and `add_executable`. Later versions lift this restriction, but we need to adhere to it.
> I meant TARGET_OBJECTS, not TARGET_SOURCES.
Although I guess it's unclear if passing the generator expression to `target_sources` will end up having the same effect as listing it in the sources list directly in the `add_library` call (and would therefore be okay on older CMake versions). Can you test this with a CMake version below 3.9 (which lifted the restriction)?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60166