Review Request: CPP: build switch context, give "case <cursor>" special completion

David Nolden david.nolden.kde at art-master.de
Sat Jun 18 11:30:03 UTC 2011


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/101671/#review3994
-----------------------------------------------------------


Nice work, but needs some more tweaks to fix the deadlocks.


languages/cpp/codecompletion/context.cpp
<http://git.reviewboard.kde.org/r/101671/#comment3279>

    This will cause deadlocks (see the comment at the top of foregroundlock.h), because the duchain-lock is held.
    
    Qt-4.8 won't help too, as it will cause the same deadlock.
    
    You can use the ForegroundLock to get the required information, but you have to move the code out of the context where the duchain-lock is held. You have to make sure that the duchain-objects stay valid for the time you unlock the duchain-lock, you can use IndexedDeclaration or IndexedDUContext or similar for that.
    
    As you have to access the foreground synchronously anyway, you don't need to change DocumentAccess. You can simply do "PersistentMovingRange r = context->createMovingRange(); unlock duchain; ForegroundLock lock; text = r->text();" to get the text



languages/cpp/cpputils.h
<http://git.reviewboard.kde.org/r/101671/#comment3278>

    As such a mechanism would inherently have the same drawbacks as the foregroundlock (eg. possible deadlocks), this comment is not required. The idea of the "DocumentAccess" object is to perform some action asynchronously _later_ in foreground, to prevent threading problems. By performing actions synchronously, you will cause deadlocks (see later comments)


- David


On June 18, 2011, 4:58 a.m., Olivier Jean de Gaalon wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101671/
> -----------------------------------------------------------
> 
> (Updated June 18, 2011, 4:58 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Summary
> -------
> 
> Build a context for the switch AST.
> Add a new "case" parent context for cpp code completion to match the type of the switch's expression.
> Add test.
> 
> 
> This addresses bug 184185.
>     http://bugs.kde.org/show_bug.cgi?id=184185
> 
> 
> Diffs
> -----
> 
>   languages/cpp/codecompletion/context.h 08049b8 
>   languages/cpp/codecompletion/context.cpp c0d6841 
>   languages/cpp/cppduchain/contextbuilder.h 3ab9cdd 
>   languages/cpp/cppduchain/contextbuilder.cpp b00c9c8 
>   languages/cpp/cpputils.h b0b633e 
>   languages/cpp/cpputils.cpp 6ac9414 
>   languages/cpp/tests/test_cppcodecompletion.h 7546f7e 
>   languages/cpp/tests/test_cppcodecompletion.cpp 6eb4257 
> 
> Diff: http://git.reviewboard.kde.org/r/101671/diff
> 
> 
> Testing
> -------
> 
> Usage, new test.
> 
> 
> Thanks,
> 
> Olivier Jean de
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20110618/567eb857/attachment.html>


More information about the KDevelop-devel mailing list