[libcxx-commits] [PATCH] D68791: [libc++] Fix linker script generation

Thu Oct 10 21:56:12 PDT 2019

phosek marked an inline comment as done.
phosek added inline comments.

================
Comment at: libcxx/cmake/Modules/DefineLinkerScript.cmake:34
     foreach(lib IN LISTS interface_libs)
-      if (TARGET "${lib}")
+      if (TARGET "${lib}" OR
+          (${lib} MATCHES "cxxabi(_static|_shared)?" AND HAVE_LIBCXXABI) OR
----------------
phosek wrote:
> ldionne wrote:
> > ldionne wrote:
> > > How can it be that `${lib}` is equal to `cxxabi_static` or `cxxabi_shared`, but it's not a target? The same applies to the `unwind_xxx` targets. I think this has to do with how you configure your build for Fuchsia with `HAVE_LIBCXXABI`. Can you show me exactly what arguments are used in the CMake invocation?
> > > 
> > > TBH, I'd like to better understand the purpose of `HAVE_LIBCXXABI` and `HAVE_LIBUNWIND`, since those certainly look like slight hacks that would be nice to support better.
> > However, feel free to commit this for the time being in order to fix your build. But I'd really like to have the discussion about `HAVE_LIBCXXABI`, since I've been meaning to have it for a while.
> The issue is that when building these in the runtimes build, they get added in whatever order user specifies on the command-line so by the time we're processing libc++, libc++abi and libunwind may have not yet been processed so `if(TARGET "${lib})` is going to fail. D68833 is an alternative solution that's trying to address this issue more generally.
Based on the discussion in D68833, it seems like a proper solution is going to require more discussion, so I'm going to land this to fix our toolchain build and work on cleaner solution afterwards.

Repository:
  rCXX libc++

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

https://reviews.llvm.org/D68791