D6905: clang: fix precompiled preamble cache misses

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.


  fix it then push it - thanks!

INLINE COMMENTS

> context.cpp:784
> +    {
> +        ForegroundLock lock;
> +        otherUnsavedFiles = ClangUtils::unsavedFiles();

ugh - I hoped to not use the foreground lock in kdev code in order to remove it eventually. But fair enough for now, leave it as is.

> kfunk wrote in parsesession.cpp:186
> Looks good to me. CXXChainPCH seems deprecated anyway:
> 
>   CXTranslationUnit_CXXChainedPCH 	
>   DEPRECATED: Enabled chained precompiled preambles in C++.
>   
>   Note: this is a temporary option that is available only while we are testing C++ precompiled preamble support. It is deprecated.

any idea why we added this in the first place? can you also add something to your commit message saying why you remove this now? it seems somewhat unrelated to the rest (?)

> kfunk wrote in parsesession.cpp:198
> Good find.
> 
> That needs libclang 3.9 though, if I understand correctly. I'm fine with bumping the dependency in master though.

yeah, better add the ifdef?

> clangutils.cpp:70
> +        }
> +        if ( !textDocument->isModified() ) {
> +            continue;

remove spaces inside parens

> clangutils.cpp:73
> +        }
> +        ret << UnsavedFile(textDocument->url().toLocalFile(), textDocument->textLines(textDocument->documentRange()));
> +    }

split onto two lines, i.e. after the first comma

> clangutils.h:73
>  
> +    KDEVCLANGPRIVATE_EXPORT QVector<UnsavedFile> unsavedFiles();
> +

please add apidox and note that this must be done from the foreground / have the foreground locked

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D6905

To: brauch, kfunk, mwolff, kdevelop-devel
Cc: geetamc, Pilzschaf, akshaydeo, surgenight, arrowdodger
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170726/b4d92389/attachment.html>