Review Request: Make temporary DUContext for default template parameters obsolete

[Milian Wolff]

> On Oct. 7, 2012, 5:51 p.m., David Nolden wrote:
> > Looks good.
> > 
> > The shared counters were indeed annoying. I just hope that the performance doesn't suffer from the thread-local storage.

ps: could you maybe comment on the two "issues" I raised above? I.e. why is the very first explicit template argument handled specially? Why do we skip CppTemplateParameterType ?


- Milian


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


On Oct. 7, 2012, 4:50 p.m., Milian Wolff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106757/
> -----------------------------------------------------------
> 
> (Updated Oct. 7, 2012, 4:50 p.m.)
> 
> 
> Review request for KDevelop and David Nolden.
> 
> 
> Description
> -------
> 
> Use a thread-local QMultiHash<IndexedQualifiedIdentifier, IndexedType> lookup table to support default template parameters. This obsoletes the necessity of the temporary DUContext which may introduce instabilities as described in bug 297133.
> 
> One big caveat was cloning template declarations which contain an internal context. This resulted in both, the original and clone to reference the same internal context. Upon destruction of the clone, the ownership of the internal context was tried to be changed. This can crash, as it could happen while only holding a read lock, yet the referenced internal context is in the DUChain and thus must only be altered while holding a write lock.
> 
> This change also makes it possible to remove the code surrounding Declaration::clone and Declaration::clonePrivate, as this was the only places which actually used it. Considering the major pitfalls and caveats of this API, I say this is a very good thing.
> 
> Furthermore, while introducing the thread-local data, I fixed the two safety recursion counters: Previously they where shared between threads which does not make any sense when we want to count the recursion depth that is thread-specific.
> 
> 
> This addresses bug 297133.
>     http://bugs.kde.org/show_bug.cgi?id=297133
> 
> 
> Diffs
> -----
> 
>   languages/cpp/cppduchain/templatedeclaration.cpp e423693 
>   languages/cpp/cppduchain/templateparameterdeclaration.h 9fb44aa 
>   languages/cpp/cppduchain/templateparameterdeclaration.cpp 08b76bb 
> 
> Diff: http://git.reviewboard.kde.org/r/106757/diff/
> 
> 
> Testing
> -------
> 
> All existing unit tests pass. KDE Rocs and boost/proto/matches.hpp get parsed without crashing.
> 
> 
> Thanks,
> 
> Milian Wolff
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20121008/144c236a/attachment.html>