<div dir="ltr">(Not sure if we should continue the discussion here or in the review.  I think the mailing list makes more sense because cfe-dev is likely read by more people than cfe-commits).<div><br></div><div>I want to clarify that I mixed terms in a misleading way.  By "Clang tools", I meant Clang-based out-of-tree tools (that currently need to use libclang.so or libclang*.a).  So the new shared library will be similar to the existing libclang.so.</div><div><br></div><div class="gmail_quote"><div dir="ltr">I'm not sure if there's enough value in linking tools in clang/tools and clang/tools/extra with a common single shared object.  If there is, I'll be willing to implement those as well.  This will ultimately benefit us as well, by making the toolchains maintained by our team smaller.</div><div dir="ltr"><br></div><div dir="ltr">On Wed, Sep 12, 2018 at 11:52 AM <<a href="mailto:kristina@nym.hush.com" target="_blank">kristina@nym.hush.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Here's my edited comment from the differential review, I think it's a great <br>
idea but there are fundamental flaws and inconsistency with it:<br>
<br>
<br>
I'm in support as long as it's a configuration option defaulting similar to<br>
LLVM's one. Should likely follow the same naming convention as LLVM, ie. clang-<br>
shlib. Clang has a lot of tools it ships with especially if you consider extras,<br>
I think this is one of the cases where an exception for libClang-8.so can be<br>
made as long as it's not a default. It would make builds of things like clang-<br>
tidy much faster without requiring a fully shared library build.<br>
<br>
IIRC libclang is a tool in itself, and is not mandatory for Clang driver tool<br>
build which is the most fundamental part to most customers, while this new<br>
library is not. There are a lot of things the downstream discussions have not<br>
covered it seems since this only accounts for for a full<br>
Clang+Tools+ARCMT+StaticAnalyzer+Extra (ie. tidy, include fixer) build. This<br>
just fails to account for so much. The pairing of libclang and the new shared<br>
library defeats the entire point of it altogether, since libclang is not<br>
required by the driver. Nor by most in-tree tools.<br>
<br>
<br>
On 12/09/2018 at 6:57 PM, "Pirama Arumuga Nainar via cfe-dev" <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br>
><br>
>Hi Ivan,<br>
><br>
>Thanks for pointing out Clang plugins as a viable alternative for <br>
>my<br>
>proposal.  We had considered writing the tools as a plugin but <br>
>decided<br>
>against it - due to some rough edges with Android's build system.<br>
><br>
>For now, we decided to carry build rules for the proposed library <br>
>in<br>
>downstream.  I've updated my change <br>
>(<a href="https://reviews.llvm.org/D50359" rel="noreferrer" target="_blank">https://reviews.llvm.org/D50359</a>) to<br>
>make it more general, and will leave it open in case there's more <br>
>interest<br>
>to revive it in the future.<br>
><br>
>Thanks,<br>
>-Pirama<br>
><br>
>On Mon, Aug 6, 2018 at 11:28 PM Ivan Donchevskii <br>
><<a href="mailto:ivan.donchevskii@qt.io" target="_blank">ivan.donchevskii@qt.io</a>><br>
>wrote:<br>
><br>
>> Hi!<br>
>><br>
>> I think I meant CLANG_PLUGIN_SUPPORT flag which enables<br>
>> export_executable_symbols(clang) in<br>
>> tools\clang\tools\driver\CMakeLists.txt<br>
>><br>
>> Probably in combination with LLVM_EXPORT_SYMBOLS_FOR_PLUGINS.<br>
>><br>
>><br>
>> Ivan<br>
>> ------------------------------<br>
>> *From:* Pirama Arumuga Nainar <<a href="mailto:pirama@google.com" target="_blank">pirama@google.com</a>><br>
>> *Sent:* Monday, August 6, 2018 10:53:29 PM<br>
>> *To:* Ivan Donchevskii; cfe-dev list<br>
>> *Cc:* Tzu-hsiang Chien; Stephen Hines<br>
>> *Subject:* Re: [cfe-dev] libclang shared library that exports <br>
>all symbols<br>
>><br>
>><br>
>><br>
>> On Fri, Aug 3, 2018 at 1:37 AM Ivan Donchevskii <br>
><<a href="mailto:ivan.donchevskii@qt.io" target="_blank">ivan.donchevskii@qt.io</a>><br>
>> wrote:<br>
>><br>
>> Hi!<br>
>> Why do you need to mix C++ symbols with what libclang exports?<br>
>><br>
>><br>
>> My reasoning was that the C/C++ difference was already imposed <br>
>by the<br>
>> headers included by a libclang.so user.  But I also see why <br>
>limiting the<br>
>> exported symbols can catch user errors.<br>
>><br>
>><br>
>> Doesn't it make sense to have separate clang.so instead? I also <br>
>think<br>
>> there's already a possibility to build such library by providing <br>
>some LLVM<br>
>> flags.<br>
>><br>
>><br>
>> I think you mean LLVM_ENABLE_SHARED?  That builds each <br>
>individual library<br>
>> (libclangAST, libclangDriver etc) as shared libraries, rather <br>
>than<br>
>> producing a single shared library.  I am not aware of any other <br>
>option that<br>
>> can help.<br>
>><br>
>> To clarify my proposal, I've uploaded a patch (<br>
>> <a href="https://reviews.llvm.org/D50359" rel="noreferrer" target="_blank">https://reviews.llvm.org/D50359</a>) that adds this new library.  <br>
>The library<br>
>> is named libclang-cxx in that patch - which I think is slightly <br>
>more<br>
>> informative than libclang-full.<br>
>><br>
>> Please add comments in the patch or to this discussions.<br>
>><br>
>><br>
>> Kind regards,<br>
>><br>
>> Ivan<br>
>> ------------------------------<br>
>> *From:* cfe-dev <<a href="mailto:cfe-dev-bounces@lists.llvm.org" target="_blank">cfe-dev-bounces@lists.llvm.org</a>> on behalf of <br>
>Pirama<br>
>> Arumuga Nainar via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>><br>
>> *Sent:* Thursday, August 2, 2018 11:48:16 PM<br>
>> *To:* cfe-dev list<br>
>> *Cc:* Tzu-hsiang Chien<br>
>> *Subject:* Re: [cfe-dev] libclang shared library that exports <br>
>all symbols<br>
>><br>
>> Ping for any thoughts on this proposal...<br>
>><br>
>> On Thu, Jul 26, 2018 at 3:59 PM Pirama Arumuga Nainar <br>
><<a href="mailto:pirama@google.com" target="_blank">pirama@google.com</a>><br>
>> wrote:<br>
>><br>
>> libclang.so exports only the symbols needed by the C API.  This <br>
>is in<br>
>> contrast to libLLVM.so that exports all symbols from the LLVM <br>
>static<br>
>> libraries.  Would it be useful to provide a libclang shared <br>
>library that<br>
>> exports all symbols for use by tools that use Clang's (admittedly<br>
>> non-backwards-compatible) C++ API?<br>
>><br>
>> We can either:<br>
>> 1. Add a new shared library (libclang_full.so?) that is built <br>
>based on a<br>
>> CMake option.<br>
>> 2. Export all symbols from the current libclang.so.  We'd have <br>
>to also<br>
>> include a few additional libraries such as libClangAnalysis.  <br>
>Tools using<br>
>> the C API are still restricted to the stable interface if they <br>
>use the<br>
>> clang-c headers.<br>
>><br>
>> Motivation:<br>
>> There are a few Clang-based tools used by Android's build system <br>
>(1, 2)<br>
>> that use the C++ API.  They are built using Android build rules <br>
>but need to<br>
>> link against Android's Clang toolchain that's built with CMake.  <br>
>We don't<br>
>> want to include the libclang static libraries with the toolchain <br>
>for space<br>
>> considerations, and also to avoid exposing Clang's build <br>
>internals<br>
>> (internal library dependences and changes to them) to downstream <br>
>tools.<br>
>><br>
>> Do other Clang/LLVM toolchain maintainers face similar issues <br>
>and have<br>
>> solutions that are applicable here?<br>
>><br>
>> [1]<br>
>> <br>
><a href="https://android.googlesource.com/platform/development/+/master/vndk" rel="noreferrer" target="_blank">https://android.googlesource.com/platform/development/+/master/vndk</a><br>
>/tools/header-checker/<br>
>> [2]<br>
>> <br>
><a href="https://android.googlesource.com/platform/development/+/master/vndk" rel="noreferrer" target="_blank">https://android.googlesource.com/platform/development/+/master/vndk</a><br>
>/tools/vtable-dumper/<br>
>><br>
>><br>
<br>
</blockquote></div></div>