DavidSpickett added a comment.
+1 from me also, but someone else should check that this is a reasonable way to
implement it cmake wise (not that this is a horrible hack but I never can tell
with cmake).
One more question, does this same issue potentially apply to llvm-tblgen and
has that got any special cmake changes to account for it? (if it doesn't, leave
it as is, but as a comparison point)
================
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).
----------------
nhaehnle wrote:
> 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?
That would work, otherwise it seems like a thing that sometimes happens under
conditions that aren't explained.
================
Comment at: clang/lib/Support/CMakeLists.txt:26
+ DISABLE_LLVM_LINK_LLVM_DYLIB
+ ${clangSupport_sources})
+endif()
----------------
nhaehnle wrote:
> 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.
> Does that answer your questions?
Yes but I don't think I have the expertise to say this is a good way to
implement this change.
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
