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

This revision was automatically updated to reflect the committed changes. Closed by commit rGdce78646f07f: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport (authored by nhaehnle). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134637/ne

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

mstorsjo accepted this revision. mstorsjo added a comment. This revision is now accepted and ready to land. LGTM, this looks reasonable to me. (Although wait a little bit more if someone else responds to the ping this time...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

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

nhaehnle added a comment. Ping^2 after ~a week. The previous discussion seemed quite favorable, so is this just a case of everybody being too afraid to say "Yes" because this is a dark corner that few people ever look at? ;) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

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

nhaehnle added a comment. Gentle ping :) If there are no objections, I'd like to commit this in a few days. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134637/new/ https://reviews.llvm.org/D134637 ___

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

nhaehnle updated this revision to Diff 467550. nhaehnle added a comment. - use the object library version of clangSupport if available Thank you for the pointers. Using this object library makes a lot of sense to me and it does appear to work. Judging by AddClang.cmake though, the object library

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

phosek added a comment. In D134637#3835935 , @nhaehnle wrote: > Thank you all for the reviews. I've integrated the suggestions except for: > >> A possible alternative solution would be to build clangSupport_sources as an >> object library, and then link

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

nhaehnle added a comment. Thank you all for the reviews. I've integrated the suggestions except for: > A possible alternative solution would be to build clangSupport_sources as an > object library, and then link that library into clangSupport and clang-tblgen > which could be done unconditional

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

nhaehnle updated this revision to Diff 465284. nhaehnle added a comment. Remove the confusing "accidentally" Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134637/new/ https://reviews.llvm.org/D134637 Files: clang/lib/Support/CMakeLists.txt cla

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

nhaehnle updated this revision to Diff 465283. nhaehnle added a comment. - define an alias so that clang-tblgen can always link against "clangSupport_tablegen" - use add_llvm_library(... BUILDTREE_ONLY ...) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

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

phosek added a comment. A possible alternative solution would be to build `clangSupport_sources` as an object library, and then link that library into `clangSupport` and `clang-tblgen` which could be done unconditionally; the advantage is that you don't need to compile `clangSupport_sources` tw

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

beanz added a comment. I think this approach mostly looks sane to me. @phosek, and @Ericson2314 may have different feedback. Comment at: clang/lib/Support/CMakeLists.txt:23 + # libLLVM-*.so). + llvm_add_library(clangSupport_tablegen +STATIC Unless there

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

aaron.ballman added reviewers: phosek, Ericson2314. aaron.ballman added a comment. Adding the CMake code owners as reviewers for input on this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134637/new/ https://reviews.llvm.org/D134637

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

mstorsjo added a reviewer: beanz. mstorsjo added a subscriber: beanz. mstorsjo added a comment. Adding @beanz who might have some more cmake knowledge on whether this is the best/least bad way of doing things. Comment at: clang/lib/Support/CMakeLists.txt:26 +DISABLE_LLVM_L

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

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 a

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

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). D

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

DavidSpickett 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). -

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

mstorsjo added a comment. Nice, +1 this looks reasonable to me. I've run into similar issues occasionally with mingw builds with dylib enabled too, where (depending on how the linker ends up pulling in objects) you can end up with duplicate definitions etc. I would expect that this also fixes t

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

nhaehnle created this revision. nhaehnle added reviewers: kito-cheng, khchen, MaskRay, aaron.ballman, DavidSpickett. Herald added subscribers: StephenFan, kristof.beyls. Herald added a project: All. nhaehnle requested review of this revision. Herald added a project: clang. Herald added a subscribe