[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

[Nicolai Hähnle via Phabricator via cfe-commits]

nhaehnle added inline comments.


================
Comment at: clang/lib/Support/CMakeLists.txt:21
+  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
+  # accidentally link against libLLVMSupport twice (once statically and once via
+  # libLLVM-*.so).
----------------
DavidSpickett wrote:
> Without this change, is it the case that it will always link against libLLVMSupport twice with the DYLIB conifg?
> 
> "accidentally" sounds like you could stumble into it but from what I see, it's always been doing this and once your other change lands, it would always result in an error.
> ```
> This is so clang-tblgen doesn't link against libLLVMSupport twice (once statically and once via libLLVM-*.so).
> ```
I meant "accidentally" in the sense that *-tblgen isn't supposed to link against libLLVM-*.so, but ended up doing so after clangSupport was added earlier this year. Perhaps I should just remover the adverb?


================
Comment at: clang/lib/Support/CMakeLists.txt:26
+    DISABLE_LLVM_LINK_LLVM_DYLIB
+    ${clangSupport_sources})
+endif()
----------------
DavidSpickett wrote:
> Can you detail which targets link to/include what and how the problem happens? I'm trying to understand why we can't just use `DISABLE_LLVM_LINK_LLVM_DYLIB` on its own here.
clangSupport is included by clang-tblgen but also by libclangcpp. The underlying idea is that of all the users of clangSupport, clang-tblgen is special because it uses the DISABLE_LLVM_LINK_LLVM_DYLIB override.

clangSupport links against Support, which becomes a link against libLLVM-*.so with LLVM_LINK_LLVM_DYLIB=ON. So, in an LLVM_LINK_LLVM_DYLIB=ON build, we get with this change:

- clangSupport links against Support, which becomes dynamically linking against libLLVM-*.so (this is unchanged)
- clangSupport_tablegen links against Support statically
- clang-tblgen links against clangSupport_tablegen (and also directly against Support) statically
- other users of clangSupport link against clangSupport somehow, and then transitively dynamically against libLLVM-*.so

Does that answer your questions?

Specifically, if we were to just add DISABLE_LLVM_LINK_LLVM_DYLIB to clangSupport, then we risk a situation where some other user of clangSupport links against Support twice; once via the copy of Support that is statically linked to from clangSupport; and once via libLLVM-*.so that gets pulled in via other dependencies. To be honest, I don't know for certain whether that is a problem that would happen, but it seemed likely enough to me that I wouldn't want to risk it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134637