[PATCH] D68833: [CMake] Re-order runtimes in the order of dependencies

[Louis Dionne via Phabricator via llvm-commits]

ldionne accepted this revision.
ldionne added a comment.

CMake tracks dependencies between targets, but not between directories. If the CMakeLists.txt in some directory (e.g. `<root>/libcxx/`) needs a target defined in another directory (e.g. `<root>/libcxxabi/`), one has to make sure that `libcxxabi`'s `CMakeLists.txt` is included before `libcxx`'s `CMakeLists.txt`. This isn't new or vexing, IMO. If that is what this patch ensures (I don't know the runtimes build very well), I think this is good.

>   [...] in general we should avoid using `if(TARGET...)`.

I'd like to question that affirmation. What is it based on?

> The other runtime libraries should be able to use the `HAVE_${runtime}` variables to determine the presence of the other runtimes

IMO, the `HAVE_${runtime}` variables are the weird ones here. The normal LLVM monorepo build orders the directories correctly, and we don't run into that issue. I don't see how `HAVE_${runtime}` can get around things like being able to query properties of e.g. `cxxabi_shared` inside the libcxx build before `cxxabi_shared` has been defined. I do support a push towards using generator expressions more, but I don't think generator expressions are a complete solution to this problem.

I'd like to see this patch go in under some form so that we can remove the hacky workaround introduced in https://reviews.llvm.org/D68791.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68833