[cfe-dev] Cross Translation Unit Support in Clang

Wed Jul 26 00:12:44 PDT 2017

On Tue, Jul 25, 2017 at 11:15 PM Vassil Vassilev via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

>
> On 20/07/17 09:55, Dániel Krupp via cfe-dev wrote:
>
> Hi,
>
>
>
> this is the patch with the new library:
>
>
>
> Add preliminary Cross Translation Unit support library
>
> https://reviews.llvm.org/D34512
>
>
>
> we are blocked until this is not approved.
>
> One generic question maybe not very related to this patch. Have you guys
> considered using the module index infrastructure to express the cross-TU
> mappings?
>
> IIRC its original idea was to return information about which module
> declares a decl. That works well for C/ObjC and I am not sure what's its
> state for C++.
>

This is an abstraction level higher, though; we need an interface to get
cross-TU information regardless of how the code is compiled (most C++ is
compiled without modules).

> Thanks & Regards,
>
> Daniel
>
>
>
> *From:* ganna at apple.com [mailto:ganna at apple.com <ganna at apple.com>]
> *Sent:* 2017. július 20. 8:40
> *To:* Dániel Krupp <daniel.krupp at ericsson.com> <daniel.krupp at ericsson.com>
> *Cc:* Manuel Klimek <klimek at google.com> <klimek at google.com>; Gábor
> Horváth <xazax.hun at gmail.com> <xazax.hun at gmail.com>; Richard Smith
> <richard at metafoo.co.uk> <richard at metafoo.co.uk>; cfe-dev at lists.llvm.org
> *Subject:* Re: [cfe-dev] Cross Translation Unit Support in Clang
>
>
>
> Hi Daniel,
>
>
>
> On Jul 17, 2017, at 7:02 AM, Dániel Krupp via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>
>
> Hi Richard,
>
>
>
> as you suggested we factored out the Cross Translation Unit feature into a
> separate library:
>
> https://reviews.llvm.org/D30691
>
>
>
> Is the addition of the separate library done in its own patch? The link
> above just points to the Cross TU static analyzer patch.
>
>
>
> called “CrossTU”.
>
>
>
> We are waiting for your approval for the name of the library.
>
> This is blocking our work on upstreaming the Cross TU function in Clang
> Static Analyzer (https://reviews.llvm.org/D30691).
>
>
>
> Could you please take a look?
>
>
>
> Thanks,
>
> Daniel
>
>
>
>
>
> *From:* cfe-dev [mailto:cfe-dev-bounces at lists.llvm.org
> <cfe-dev-bounces at lists.llvm.org>] *On Behalf Of *Manuel Klimek via cfe-dev
> *Sent:* 2017. július 6. 11:34
> *To:* Gábor Horváth <xazax.hun at gmail.com>; Richard Smith <
> richard at metafoo.co.uk>
> *Cc:* Clang Dev <cfe-dev at lists.llvm.org>
> *Subject:* Re: [cfe-dev] Cross Translation Unit Support in Clang
>
>
>
> 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
> <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
>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
>
>
> _______________________________________________
> cfe-dev mailing listcfe-dev at lists.llvm.orghttp://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
> _______________________________________________
> 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/20170726/746ea16b/attachment.html>