[cfe-dev] Cross Translation Unit Support in Clang
Vassil Vassilev via cfe-dev
cfe-dev at lists.llvm.org
Tue Jul 25 14:15:39 PDT 2017
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++.
>
> Thanks & Regards,
>
> Daniel
>
> *From:*ganna at apple.com [mailto:ganna at apple.com]
> *Sent:* 2017. július 20. 8:40
> *To:* Dániel Krupp <daniel.krupp at ericsson.com>
> *Cc:* Manuel Klimek <klimek at google.com>; Gábor Horváth
> <xazax.hun at gmail.com>; Richard Smith <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 <mailto: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]*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
> <mailto:xazax.hun at gmail.com>>; Richard Smith
> <richard at metafoo.co.uk <mailto:richard at metafoo.co.uk>>
> *Cc:*Clang Dev <cfe-dev at lists.llvm.org
> <mailto: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
> <mailto:xazax.hun at gmail.com>> wrote:
>
> On 5 July 2017 at 23:09, Richard Smith <richard at metafoo.co.uk
> <mailto:richard at metafoo.co.uk>> wrote:
>
> On 26 June 2017 at 07:16, Manuel Klimek <klimek at google.com
> <mailto: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
> <mailto: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 <mailto:ganna at apple.com>> wrote:
>
> On Jun 23, 2017, at 12:39 PM, Gábor
> Horváth <xazax.hun at gmail.com
> <mailto: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 <mailto:ganna at apple.com>>
> wrote:
>
> On Jun 23, 2017, at 1:40 AM, Gábor
> Horváth <xazax.hun at gmail.com
> <mailto: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
> <mailto: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
> <mailto: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 <mailto: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 <mailto: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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20170725/ca040d04/attachment.html>
More information about the cfe-dev
mailing list