[libcxx-commits] [PATCH] D113467: [libcxx] Add a dependency on unwind in cxx_static, if LIBCXXABI_USE_LLVM_UNWINDER is set

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/src/CMakeLists.txt:289-294
+  if (LIBCXXABI_USE_LLVM_UNWINDER)
+    # If we intend to use the just-built unwinder, add a dependency so that it
+    # gets built, even if we technically aren't going to link it in at this
+    # stage.
+    add_dependencies(cxx_static unwind)
+  endif()
----------------
mstorsjo wrote:
> ldionne wrote:
> > Should we put that dependency on `check-cxx` instead? We could add it to `cxx-test-depends`.
> That was my first idea, but in some sense, this felt more natural and symmetrical to adding the unwind dependency to `cxx_shared` right above here - so you get libunwind built if you have `LIBCXXABI_USE_LLVM_UNWINDER` set, when you do `ninja cxx`, even if you've only set up a static libcxx.
> 
> But I don't feel strongly, I can post that version of the patch as well.
Ok, I guess I am swayed by the symmetry argument with `cxx_shared`. Thanks for taking the time to explain. Let's go with this one and close D113481.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113467