[cfe-dev] Cross Translation Unit Support in Clang

Richard Smith via cfe-dev cfe-dev at lists.llvm.org
Wed Jul 5 14:09:52 PDT 2017


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

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.

3: The above binary size increases seem acceptable for a significant
feature that the static analyser needs.


> 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/20170705/226c7840/attachment.html>


More information about the cfe-dev mailing list