phosek added inline comments.
================
Comment at: libcxx/lib/CMakeLists.txt:269
+ AND (TARGET cxxabi_static OR HAVE_LIBCXXABI))
+ #if ((TARGET ${LIBCXX_CXX_ABI_LIBRARY}) OR
+ #(${LIBCXX_CXX_ABI_LIBRARY} MATCHES "cxxabi(_static|_shared)?" AND
HAVE_LIBCXXABI))
----------------
ldionne wrote:
> phosek wrote:
> > ldionne wrote:
> > > I don't understand why any of this needs to change -- can you please
> > > explain? Also, you probably didn't mean to leave the commented-out lines.
> > The reason this change is needed the case when we're linking shared
> > libc++abi into shared libc++ in which case `${LIBCXX_CXX_ABI_LIBRARY}` will
> > be set to `cxxabi_shared` in `HandleLibCXXABI.cmake` but we cannot merge
> > `libc++abi.so` into `libc++.a`, so instead we force the use of
> > `cxxabi_static` here.
> >
> > Alternatively, we could modify `HandleLibCXXABI.cmake` to set two
> > dependencies, one for the static case and one for the shared case and use
> > the former one here.
> >
> > Removed the commented out lines.
> Thanks. There's something I still don't understand. If you are linking the
> ABI library dynamically, why would you want to merge it (well, the static
> version of it) into `libc++.a`? It seems like this is somewhat defeating the
> purpose of dynamically linking against the ABI library, no?
In our case, we want to link shared library dynamically and merge all static
libraries, so `LIBCXX_STATICALLY_LINK_ABI_IN_SHARED_LIBRARY=OFF` and
`LIBCXX_STATICALLY_LINK_ABI_IN_STATIC_LIBRARY=ON`. This means that
`LIBCXX_CXX_ABI_LIBRARY` will be set to `cxxabi_shared` and it's what will be
used other places throughout this file as the interface library. However, we
cannot merge `libc++abi.so` into `libc++.a` so that's why we have to explicitly
select `cxxabi_static` here rather than simply using `LIBCXX_CXX_ABI_LIBRARY`
as before.
Like I mentioned in the previous comment, alternative would be to split this
into two variables, e.g. `LIBCXX_CXX_SHARED_ABI_LIBRARY` and
`LIBCXX_CXX_STATIC_ABI_LIBRARY`, would you prefer that approach?
================
Comment at: libcxxabi/src/CMakeLists.txt:64
# FIXME: Is it correct to prefer the static version of libunwind?
if (NOT LIBCXXABI_ENABLE_STATIC_UNWINDER AND (TARGET unwind_shared OR
HAVE_LIBUNWIND))
list(APPEND LIBCXXABI_LIBRARIES unwind_shared)
----------------
ldionne wrote:
> Does this not need to change anymore? I think we'd have to set different
> flags for `cxxabi_shared` and `cxxabi_static`.
>
This is relying on the fact that `LIBCXXABI_LIBRARIES` isn't used for library
merging that's done on lines 158-162 and CMake should ignore shared library
dependencies passed to `target_link_libraries` when building static library as
done on line 163 (since linking dynamic library target against static library
doesn't make sense). This seems to be working fine in my testing, but I'm also
OK splitting this into two variables if you want it to be explicit.
Repository:
rCXX libc++
https://reviews.llvm.org/D49502
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
