Review Request 121757: Use target environment whenever available, and implement TU pinning (WIP)

[Sergey Kalinichev]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121757/#review73567
-----------------------------------------------------------



clangparsejob.cpp
<https://git.reviewboard.kde.org/r/121757/#comment51218>

    Why is there 2 abortRequested() checks in a row?
    Is it really necessarily?



clangparsejob.cpp
<https://git.reviewboard.kde.org/r/121757/#comment51220>

    That looks weird for me. Here if(abortRequested) then existingSessionData() returns nullptr. But the code above doesn't know about it, and keeps on creating new session data.



clangparsejob.cpp
<https://git.reviewboard.kde.org/r/121757/#comment51221>

    And what about isUpdateRequired?
    The old code returned from ClangParseJob::run immeditely, now you recreating sessionData instead. Are you sure that's how it should be?


Also, just tested it. Seems like it doesn't work when e.g. first I opened clangparsejob.cpp, then clangparsejob.h
Closed KDevelop (without closing clangparsejob.cpp). Opened again. Tried to edit clangparsejob.h Now it can't find include paths for it.
But if I first do f5 in the source file then include path in the header will be resolved

- Sergey Kalinichev


On Jan. 9, 2015, 11:20 a.m., Olivier Jean de Gaalon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121757/
> -----------------------------------------------------------
> 
> (Updated Jan. 9, 2015, 11:20 a.m.)
> 
> 
> Review request for KDevelop, Kevin Funk and Milian Wolff.
> 
> 
> Repository: kdev-clang
> 
> 
> Description
> -------
> 
> Note: This requires IBuildSystemManager::hasIncludesOrDefines, which should probably be renamed to isTranslationUnit()
> 
> The idea here is to use the AST of the buildsystem's translation units wherever possible, since that's the only file that's guaranteed to have correct defines and includes (and furthermore can handle strange cases such as '#include inside namespace' and picky include/define ordering).
> This proposal essentially allows TU environments to override non-TU environments and then get "pinned" so that the non-TU will continue to use the TU AST to build its duchain.
> This could eventually be exposed in the UI, allowing the user to pick different "views" of headers (overriding the pinned TU).
> The pinning data needs to be fleshed out to handle expiry (header no longer included) and be serialized with an ItemRepository.
> 
> I'm looking for feedback on this idea... where does it break? Any better ideas?
> 
> This more or less works, we can flesh it out more if we want to take this route.
> 
> 
> Diffs
> -----
> 
>   clangparsejob.h 1cc8282 
>   clangparsejob.cpp 297b836 
>   duchain/clanghelpers.h 4216454 
>   duchain/clanghelpers.cpp 5f29a2a 
>   duchain/clangindex.h 991c7ca 
>   duchain/clangindex.cpp 02c93cf 
>   duchain/clangparsingenvironment.h 7bb8111 
>   duchain/clangparsingenvironment.cpp 1decc14 
>   duchain/clangparsingenvironmentfile.h 953ee94 
>   duchain/clangparsingenvironmentfile.cpp b3d0563 
>   duchain/clangpch.cpp 1ce3457 
>   duchain/parsesession.h b688fb1 
>   duchain/parsesession.cpp f41768a 
>   duchain/tuduchain.cpp 33a0570 
>   tests/test_duchain.cpp 7db9fea 
> 
> Diff: https://git.reviewboard.kde.org/r/121757/diff/
> 
> 
> Testing
> -------
> 
> Tests fail in the same manner as before
> 
> 
> File Attachments
> ----------------
> 
> kdevelop patch
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/01/02/5c0314bf-f241-4ad0-93f6-9a0a48a79135__kdevelop-hasincordef.diff
> platform patch
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/01/02/edc11881-06c6-4570-9afd-824a99687116__kdevplatform-hasincordef.diff
> 
> 
> Thanks,
> 
> Olivier Jean de Gaalon
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20150109/c9df5525/attachment-0001.html>