[cfe-dev] Cross Translation Unit Support in Clang

Manuel Klimek via cfe-dev cfe-dev at lists.llvm.org
Thu Jul 6 02:34:10 PDT 2017 

On Thu, Jul 6, 2017 at 10:57 AM Gábor Horváth <xazax.hun at gmail.com> wrote:

> On 5 July 2017 at 23:09, Richard Smith <richard at metafoo.co.uk> wrote:
>
>> On 26 June 2017 at 07:16, Manuel Klimek <klimek at google.com> wrote:
>>
>>> On Mon, Jun 26, 2017 at 4:08 PM Gábor Horváth via cfe-dev <
>>> cfe-dev at lists.llvm.org> wrote:
>>>
>>>> Hi,
>>>>
>>>> Some benchmarks of the binary size after introducing the libTooling
>>>> dependency to clang (using static linking on Linux):
>>>>
>>>> Release:
>>>> 85457072 -> 85505864
>>>> Debug:
>>>> 1777932672 -> 1779938696
>>>>
>>>> The increase is less than 1% in both cases. So, in my opinion, the
>>>> binary size is not really a problem here.
>>>> Of course, the link times might increase a bit as well.
>>>>
>>>> A question is, who should make the call whether this penalty is ok or
>>>> not?
>>>>
>>>
>>> (after replying on the patch without noticing that this thread is not
>>> part of the patch, here it goes again :)
>>> Richard (cc'ed) usually owns decisions around clang itself. Writing an
>>> email to cfe-dev with the numbers and wait for whether others have concerns
>>> would probably also be good (many will probably not continue reading this
>>> thread).
>>>
>>
>> I'm a bit lost as to what's actually being proposed here. It seems that
>> there are at least three separate questions:
>>
>> 1) Does some kind of support for a database of multiple translation units
>> belong in Clang? (The exact design of that database can be discussed
>> separately.)
>> 2) Should it be a separate library or part of libTooling?
>> 3) Is it acceptable for the clang static analyser (and thus the clang
>> binary) to link against that library?
>>
>> My thoughts:
>>
>> 1: Yes, this seems like a sensible thing to have within the clang
>> repository, particularly if there would be in-tree users of it. The clang
>> static analyser would be one justification for this; another would be
>> support of exported templates (if we ever want to be C++98
>> feature-complete). We'll need to discuss what the model is for this layer
>> (multiple ASTContexts? one ASTContext modeling multiple TUs?), and how it
>> fits in with other parts of Clang that solve similar problems (particularly
>> ExternalASTSources, the serialization layer, ASTImporter).
>>
>
> This functionality basically using the serialization layer and
> ASTImporter, and provides the user with an easy interface to import
> function definitions from other TU and merge into the current one. This
> could be extended to any kinds of definitions once users need that.
>
>
>>
>> 2: I haven't seen anyone provide good justification for merging this
>> library with libTooling, and the fact that the static analyser wants to use
>> this and doesn't want tooling support strongly suggests to me that it
>> should be separate.
>>
>
> Once we add on demand reparsing instead of loading AST files, the
> libTooling will be a dependency of this code. Moreover it does nothing
> static analyzer specific, this is the reason why we was thinking that
> libTooling is a good place for this functionality.
>
> The currently proposed (libTooling) functionality can load certain
> interesting function definitions from external source into the current AST,
> so existing single TU tools can operate on the AST with more available
> information as if they had cross TU capabilities.
>

I think we can generally break everything that deals with running clang out
of libtooling into an extra library.


> 3: The above binary size increases seem acceptable for a significant
>> feature that the static analyser needs.
>>
>
> Great news, thanks!
>
> Regards,
> Gábor
>
>
>>
>>
>>> On 23 June 2017 at 22:06, Anna Zaks <ganna at apple.com> wrote:
>>>>
>>>>>
>>>>> On Jun 23, 2017, at 12:39 PM, Gábor Horváth <xazax.hun at gmail.com>
>>>>> wrote:
>>>>>
>>>>> This is a dependency for the Static Analyzer component within clang
>>>>> (to another component, which is in clang as well).
>>>>>
>>>>>
>>>>> OK. This means that we are talking about adding a dependency on
>>>>> libTooling to clang. This would not only effect the static analyzer, so
>>>>> we’d need an OK from a greater clang community.
>>>>>
>>>>> It is a similar dependency to this:
>>>>> https://github.com/llvm-mirror/clang/commit/a994aad333a56b8c9dd49bcfb5090a393d193387
>>>>>
>>>>>
>>>>> There are 2 differences from the dependency on ASTMatchers:
>>>>>  - Nothing that is presently in libTooling us used by clang.
>>>>>
>>>>
>>>> This is actually not a difference. When the commit above was excepted,
>>>> it was the first use of ASTMatchers in the clang binary.
>>>>
>>>>
>>>>>  - The new component that the analyzer will depend on will be
>>>>> supporting a very experimental feature.
>>>>>
>>>>> The other expedient options that I can see are:
>>>>>  - Adding a separate library with just the functionality that this
>>>>> feature needs.
>>>>>  - Making the dependency conditional, following the same style as Z3
>>>>> support, and keeping it that way until the feature is fully implemented.
>>>>> This solution definitely has downsides.
>>>>>
>>>>> Yet another solution is to pull the analyzer out of clang, but
>>>>> unfortunately that is non-trivial, so I am not sure if there are volunteers
>>>>> for the task.
>>>>>
>>>>
>>>> I agree that pulling the analyzer out is a big task and would also
>>>> break the backward compatibility of the command line. So it is not the best
>>>> solution for the users.
>>>>
>>>> Regards,
>>>> Gábor
>>>>
>>>>
>>>>>
>>>>> Anna.
>>>>>
>>>>> On 23 June 2017 at 19:48, Anna Zaks <ganna at apple.com> wrote:
>>>>>
>>>>>>
>>>>>> On Jun 23, 2017, at 1:40 AM, Gábor Horváth <xazax.hun at gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>> Anna,
>>>>>>
>>>>>> Are you ok having libTooling as a dependency of the Static Analyzer?
>>>>>>
>>>>>>
>>>>>> Are we talking about introducing dependency for scan-build or clang
>>>>>> itself?
>>>>>>
>>>>>> I think this is not a bad direction since it has other good utilities
>>>>>> that the Static Analyzer could use in the future such as Replacements,
>>>>>> FixIts.
>>>>>>
>>>>>> Regards,
>>>>>> Gábor
>>>>>>
>>>>>> On 22 June 2017 at 12:10, Manuel Klimek <klimek at google.com> wrote:
>>>>>>
>>>>>>> For clang tooling, I don't really expect us to do cross-TU AST
>>>>>>> loading outside of what modules will provide, as that inherently doesn't
>>>>>>> scale. Instead, we usually design pipelined approaches where the first
>>>>>>> "scan" over the codebase provides data which are reduced to the information
>>>>>>> needed for the tool.
>>>>>>>
>>>>>>> That said, I can't predict the future, and people have expressed
>>>>>>> interest in that functionality, so
>>>>>>> a) I agree it's a good idea to add and
>>>>>>> b) I think it's a good fit for libtooling
>>>>>>>
>>>>>>> Cheers,
>>>>>>> /Manuel
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Jun 22, 2017 at 11:58 AM Aleksei Sidorin <
>>>>>>> a.sidorin at samsung.com> wrote:
>>>>>>>
>>>>>>>> Hello Gabor,
>>>>>>>>
>>>>>>>> Internally, we have created XTU module inside clang (lib/XTU). I
>>>>>>>> think
>>>>>>>> it is the best way because importing is not related to analyzer
>>>>>>>> directly. We're not going to use it outside of CSA but I think
>>>>>>>> future
>>>>>>>> users should have such possibility.
>>>>>>>>
>>>>>>>> 22.06.2017 12:41, Gábor Horváth пишет:
>>>>>>>> > Hi!
>>>>>>>> >
>>>>>>>> > It looks like there is a consensus to accept the cross translation
>>>>>>>> > unit analysis patch into the clang static analyzer:
>>>>>>>> > https://reviews.llvm.org/D30691
>>>>>>>> >
>>>>>>>> > There is one part of the patch that is independent of the static
>>>>>>>> > analyzer. The logic which can look up a function in an index and
>>>>>>>> load
>>>>>>>> > a serialized AST that contains the definition of the function.
>>>>>>>> > The lookup is cached, and after the AST is loaded, the function
>>>>>>>> > definition will be merged into the original AST.
>>>>>>>> >
>>>>>>>> > Right now, in the current patch, this functionality is in the
>>>>>>>> > ASTContext. This is definitely not the right place to put this.
>>>>>>>> So the
>>>>>>>> > question is, do you plan to utilize similar functionality in Clang
>>>>>>>> > tooling or clang tidy?
>>>>>>>> >
>>>>>>>> > If yes, we might end up creating a new module for that (or add it
>>>>>>>> to
>>>>>>>> > an existing commonly used one like libtooling?). If no, we will
>>>>>>>> move
>>>>>>>> > the corresponding code to the static analyzer.
>>>>>>>> >
>>>>>>>> > What do you think?
>>>>>>>> >
>>>>>>>> > In case you are interested in how it works, you can check out the
>>>>>>>> > EuroLLVM talk:
>>>>>>>> >
>>>>>>>> http://llvm.org/devmtg/2017-03//2017/02/20/accepted-sessions.html#7
>>>>>>>> >
>>>>>>>> > Regards,
>>>>>>>> > Gábor
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> Best regards,
>>>>>>>> Aleksei Sidorin,
>>>>>>>> SRR, Samsung Electronics
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>> cfe-dev mailing list
>>>> cfe-dev at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>>
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20170706/77c34ca7/attachment.html>

More information about the cfe-dev mailing list