Fallback to TU url when looking for a cached ParseSession.

Sergey Kalinichev kalinichev.so.0 at gmail.com
Thu Apr 23 08:28:57 UTC 2015


>The code to pick the right sessionData (i.e. the right URL) is in ClangIndex
>and reused by my patch. Or what do you mean exactly?

We only attach an AST to the URL for which we have TU, but why can't
we attach the AST for all opened in editor documents and reset the AST
pointer when session data changes instead? It'd save us from passing
ClangIndex around.

>- you have a .cpp open and parse it. The .cpp URL/DUContext gets the Clang AST
>attached
>now you open the corresponding .h file. Without my patch, you won't get any
>code completion now, even though all required information is available but
>code completion looks for the AST attached to the .h file, not the .cpp file (fixed by my >patch).

That's odd because now I do have code-completion in headers, but to
enable it I first have to modify cpp, then h, then again cpp and only
then code-completion in headers begins to work.

>Sergey, is the above with or without my patch? Do you have an example file
>pair for me to play with? What do you change to trigger the behavior?

It happens always, even without your patch. Just open any pair of
files, e.g.: tuduchain.* and see how much time clang_codeCompleteAt
takes.

>note that this might be problematic when files depend on defines or similar
>that are set in project files

But the same is valid for system headers. Still you suggest to create
PCH for STL + Qt headers. So, why it'd work for system headers, but
won't work for project headers?

--
Sergey

On 4/21/15, Milian Wolff <mail at milianw.de> wrote:
> Hey Sergey,
>
> On Tuesday 21 April 2015 11:29:54 Sergey Kalinichev wrote:
>> I've just noticed that we pass CXTranslationUnit_PrecompiledPreamble
>> flag, so that Clang can internally use PCH for preamble. Why can't we
>> build the precompiled preamble manually then, without including
>> project files? And doesn't it mean that reparse and code-completion
>> performance for cpp files will drop in that case, as Clang has to
>> reparse project files with all their includes?
>
> I don't quite understand you - can you rephrase your question? The
> CXTranslationUnit_PrecompiledPreamble option will create a temporary PCH for
> a
> .cpp that includes all the .h of that .cpp (afaik). You want to remove this
>
> option and instead manually build a PCH of all included files /except/ of
> the
> project headers? That will be quite some work, as you'll:
>
> - first parse the full TU
> - iterate over all imports and exclude project files
> - create a file only including these files
> - reparse that one to create a PCH
> - then future files can leverage this PCH
>
> note that this might be problematic when files depend on defines or similar
>
> that are set in project files
> also, one would like to share these PCHs for other TUs if possible
>
> Generally, I'd rather have a good GUI and code to generate (and update!)
> PCHs
> of system headers from within KDevelop. Afaik this is also provided by
> QtCreator and leads to significant speedups there. Imagine being able to put
>
> all of STL or all of STL + Qt into a PCH and then just using that for
> projects!
>
>> Still I don't understand why we can't create another TU for a header
>> file with includes/defines used by the cpp file? What difference does
>> it make? If it does make a difference can't we maybe create another TU
>> for a header solely for code-completion?
>
> A header is not a TU, it does not have all the required information like
> include paths and defines. The latter might even be set from other files
> included before in the TU. And doing something solely for code-completion
> means we will duplicate work: when the header changed, we'll need to reparse
>
> the TU sooner or later anyways. Also, the results will not be correct
> anymore.
>
> Bye
>
>> On 4/20/15, Olivier J. G. <olivier.jg at gmail.com> wrote:
>> > On Mon, Apr 20, 2015 at 9:44 AM, Sergey Kalinichev <
>> > kalinichev.so.0 at gmail.com> wrote:
>> > --------------------------------------------
>> >
>> >  I don't know. It feels wrong that code-completion is now responsible
>> > for
>> >
>> > such tasks as: which sessionData to choose for code-completion,
>> > especially
>> > when we
>> >
>> >  already have a very similar code in the ClangParseJob which should
>> > select
>> >
>> > the right session data for header files.
>> >
>> >     So, for me it looks like the bug is somewhere within the
>> > ClangParseJob
>> >
>> > which doesn't attach the right parse session data to header files... Or
>> > maybe I simply
>> >
>> >  don't understand how this thing suppose to work.
>> >
>> >     Also I begin to wonder whether this pin TU feature is the right
>> > thing.
>> >
>> > Because now re-parsing and code-completion in header files takes
>> > significantly more time then in cpp files. E.g. when code-completion
>> > invoked in a header file with a TU for cpp file, code completion for me
>> > can
>> > easily take 4-6 (and even more!) seconds. This is how much time it
>> > takes
>> > Clang to reparse the document (so there is probably nothing we can do
>> > about
>> > it). I believe it happens because when you edit a header file with TU
>> > for
>> > a
>> > cpp file, Clang can't use internally precompiled PCH to make a quick
>> > reparse (as it does when you edit a cpp file), so it has to reparse
>> > some
>> > (the currently edited and maybe even all other) header files and the
>> > cpp
>> > file, which takes significantly more time, than simply reparsing a
>> > header
>> > file.
>> >
>> >     So I'm actually thinking about switching back to the old approach:
>> >     each
>> >
>> > opened in editor file has it's own TU. What do you think?
>> > ---------------------------------------------
>> >
>> > To be clear: TU pinning is the /only/ correct way to handle creation of
>> > DUContexts. Only the translation units defined by the build system can
>> > be
>> > considered correct for any purpose (including code completion). If you
>> > don't keep track of a header's translation units you have no correct
>> > view
>> > of that header. This is the single most important problem with C++, and
>> > we've been fighting it since before KDevelop 4.0 was released.
>> >
>> > This affects duchain correctness, refactoring (esp. renaming),
>> > environment,
>> > etc.
>> >
>> > Obviously we have a real issue, as your analysis (breaks the
>> > precompiled
>> > preamble) is very likely correct. The /correct/ solution is to fix
>> > clang
>> > to
>> > allow us to ensure that project files do not get included in the
>> > precompiled preamble. I just don't have the time to be hacking on Clang
>> > (or
>> > KDevelop) now, but it's obvious that's where the biggest wins for
>> > KDevelop
>> > will come from at this point -- for this and other issues.
>> >
>> > With that said, in the likely case that no one is going to fix this
>> > correctly in Clang, making code completion not horribly slow in headers
>> > is
>> > obviously important. Whether it's more important than having correct
>> > TUs
>> > is
>> > a judgement call to be made by those writing the code... just keep in
>> > mind
>> > the consequences of each decision.
>> >
>> > -Olivier JG
>
> --
> Milian Wolff
> mail at milianw.de
> http://milianw.de
>


More information about the KDevelop-devel mailing list