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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
