I get that CMake does something different for PRIVATE, but the way we use CMake in LLVM we really can't make a private distinction reasonable. Because we don't sub-divide our headers per library, we don't support per-dependency include paths in LLVM or Clang's libraries.
This behavior also assumes you are using `target_include_directories` to assign PRIVATE | PUBLIC | INTERFACE to include directories, which we don't generally do in LLVM (a quick grep showed one place in LLVM code excluding our google benchmark drop). We do generally use `include_directories` which always treats things as PRIVATE, so they don't cascade to dependencies. -Chris > On Jul 12, 2019, at 7:16 PM, Shoaib Meenai <smee...@fb.com> wrote: > > I struggled for a while thinking why PRIVATE might be useful in a > target_link_libraries call for a shared library, and then I found > https://cmake.org/pipermail/cmake/2016-May/063400.html > <https://cmake.org/pipermail/cmake/2016-May/063400.html>. The relevant bit is: > > If you were paying careful attention, you would have noticed that when A > links in B as PRIVATE, the include directories of B never propagate to > something linking to A, but if A is a static library, then the *linking* of > B behaves as though the relationship was PUBLIC. This > PRIVATE-becomes-PUBLIC behaviour for static libraries only applies to the > *linking*, not to the other dependencies (compiler options/flags and > include search paths). > > So PRIVATE/INTERFACE/PUBLIC doesn’t make any difference as far as the actual > linking goes, but it does affect propagation of other options, and I think > it’s valid to want to have a PRIVATE dependency for a static library. > > From: <cbiene...@apple.com> on behalf of Chris Bieneman <be...@apple.com> > Date: Friday, July 12, 2019 at 9:08 AM > To: Shoaib Meenai <smee...@fb.com> > Cc: Alex Bradbury <a...@lowrisc.org>, cfe-commits <cfe-commits@lists.llvm.org> > Subject: Re: r365825 - [clang-shlib] Fix clang-shlib for PRIVATE dependencies > > Ah! I see what is going on here. This is kinda a silliness of CMake. > `PRIVATE` linkage for a static archive is silly, and shouldn't be possible. > All link dependencies for static archives are really `INTERFACE` > dependencies, and it looks like CMake is doing something kinda silly instead > of producing a reasonable error or warning like it probably should do. > > For context, `PRIVATE` linkage in the context of that change would mean > DirectoryWalker links something, but things that link DirectoryWalker don't. > That just isn't possible with static archives, so they become interface > dependencies (and CMake seems to insert a generator expression to make that > work). > > In `llvm_add_library` we always use `PRIVATE` linkage for shared libraries, > and `INTERFACE` for static, which does the right thing. > > Unless there is a better reason than a new patch coming in, I think the right > fix is to revert this back and expect the new patch to correct its linkage > behavior. > > -Chris > > > On Jul 12, 2019, at 8:53 AM, Shoaib Meenai <smee...@fb.com > <mailto:smee...@fb.com>> wrote: > > See https://reviews.llvm.org/D58418#1577670 > <https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D58418-231577670&d=DwMFAg&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=BaInVCyhXHMyre_oyIEnYhuQC5zsW6p65QBgZLR7ujc&s=7bhRDMtKEyVqFDlMWzo361mQT2N8lcOt-YDh-XGjZok&e=>. > More generally it would appear for any static library with a PRIVATE > dependency though. > > I guess an alternative would be to use the LINK_LIBRARIES property (which > should be free of generator expressions, I believe) to propagate dependencies > instead of INTERFACE_LINK_LIBRARIES, and just assume that no one is gonna set > an INTERFACE dependency on a static library. (Supporting PRIVATE dependencies > on a static library definitely seems more valuable than supporting INTERFACE > dependencies.) > > From: <cbiene...@apple.com <mailto:cbiene...@apple.com>> on behalf of Chris > Bieneman <be...@apple.com <mailto:be...@apple.com>> > Date: Friday, July 12, 2019 at 8:49 AM > To: Shoaib Meenai <smee...@fb.com <mailto:smee...@fb.com>> > Cc: Alex Bradbury <a...@lowrisc.org <mailto:a...@lowrisc.org>>, cfe-commits > <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> > Subject: Re: r365825 - [clang-shlib] Fix clang-shlib for PRIVATE dependencies > > One of the benefits of the object library approach for generating the clang > dylib is that it was compatible with BUILD_SHARED_LIBS. We had lots of issues > with libLLVM where people using BUILD_SHARED_LIBS would make changes that > broke it, so I was trying to make the clang dylib in a way that it could > always be enabled. > > Do we know where the nested generator expression was coming from? > > -Chris > > > > On Jul 12, 2019, at 8:32 AM, Shoaib Meenai <smee...@fb.com > <mailto:smee...@fb.com>> wrote: > > Oops, sorry about the breakage. > > Chris, aren't BUILD_SHARED_LIBS and the combined Clang dylib incompatible? > Should we disable building the latter if the former is set? > > From: Alex Bradbury <a...@lowrisc.org <mailto:a...@lowrisc.org>> > Date: Friday, July 12, 2019 at 2:02 AM > To: Shoaib Meenai <smee...@fb.com <mailto:smee...@fb.com>> > Cc: cfe-commits <cfe-commits@lists.llvm.org > <mailto:cfe-commits@lists.llvm.org>> > Subject: Re: r365825 - [clang-shlib] Fix clang-shlib for PRIVATE dependencies > > On Thu, 11 Jul 2019 at 22:20, Shoaib Meenai via cfe-commits > <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote: > > Author: smeenai > Date: Thu Jul 11 14:20:38 2019 > New Revision: 365825 > > URL: > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D365825-26view-3Drev&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=ETCU2JhfBcjWz-nbe6LUVSRQR0T1f3wCzxLIhKlMmQo&s=R9NSZG1XQDVSiE0wJUgb1kMUrG6bJkG3v5GDcTdkpAk&e= > > <https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D365825-26view-3Drev&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=ETCU2JhfBcjWz-nbe6LUVSRQR0T1f3wCzxLIhKlMmQo&s=R9NSZG1XQDVSiE0wJUgb1kMUrG6bJkG3v5GDcTdkpAk&e=> > Log: > [clang-shlib] Fix clang-shlib for PRIVATE dependencies > > Any static library with a PRIVATE dependency ends up with a > $<LINK_ONLY:...> generator expression in its INTERFACE_LINK_LIBRARIES, > which won't be evaluated by the $<TARGET_PROPERTY:...>, so we end up > with an unevaluated generator expression in the generated build file and > Ninja chokes on the dollar sign. Just use the static library directly > for its dependencies instead of trying to propagate dependencies > manually. > > Differential Revision: > https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D64579&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=ETCU2JhfBcjWz-nbe6LUVSRQR0T1f3wCzxLIhKlMmQo&s=mutN2zF1KixMEVFjNzG_Q7iVJzG5ECbrU4tX3AKWLVQ&e= > > <https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D64579&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=ETCU2JhfBcjWz-nbe6LUVSRQR0T1f3wCzxLIhKlMmQo&s=mutN2zF1KixMEVFjNzG_Q7iVJzG5ECbrU4tX3AKWLVQ&e=> > > This seems to break a -DBUILD_SHARED_LIBS build for me. It fails at > the point of linking lib/libclang_shared.so.9svn with errors like: > ld.lld: error: undefined symbol: llvm::llvm_unreachable_internal(char > const*, char const*, unsigned int) > referenced by Attributes.cpp:34 > (/home/asb/llvm-project/clang/lib/Basic/Attributes.cpp:34) > > tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/Attributes.cpp.o:(clang::attr::getSubjectMatchRuleSpelling(clang::attr::SubjectMatchRule)) > > ld.lld: error: undefined symbol: llvm::llvm_unreachable_internal(char > const*, char const*, unsigned int) > referenced by TargetCXXABI.h:168 > (/home/asb/llvm-project/clang/include/clang/Basic/TargetCXXABI.h:168) > > tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/Attributes.cpp.o:(clang::TargetCXXABI::isMicrosoft() > const) > > ld.lld: error: undefined symbol: llvm::llvm_unreachable_internal(char > const*, char const*, unsigned int) > referenced by TargetCXXABI.h:149 > (/home/asb/llvm-project/clang/include/clang/Basic/TargetCXXABI.h:149) > > tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/Attributes.cpp.o:(clang::TargetCXXABI::isItaniumFamily() > const) > > ld.lld: error: undefined symbol: llvm::EnableABIBreakingChecks > referenced by Attributes.cpp > > tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/Attributes.cpp.o:(llvm::VerifyEnableABIBreakingChecks)
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
