Review Request: Add mapping of Uses to AST nodes.

[David Nolden]

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


Generally looks not bad, just 2 things:
- The map() function is too complex. It would be better having one protected boolean member (as before) or an inline function that is very simple. The reason: The whole duchain/ast mapping is not needed in 99% of the cases, and thus it should cause a minimal runtime overhead in those cases. Also, do you really think it's needed to map only uses but not declarations, or the opposite? Either you want to do refactoring, then you can get anything mapped, or you don't want to do refactoring, then you need neither.
- In "usingDeclaration()" you don't use start_token and end_token any more. In some cases those may differ from the AST range, so those have to be used.

Generally, when changing duchain related stuff you should run the following regression tests and make sure everything still passes:
kdevelop/languages/cpp/cppduchain/tests/duchaintest
kdevelop/languages/cpp/cppduchain/tests/expressionparsertest
kdevelop/languages/cpp/tests/cppcodecompletiontest
kdevelop/languages/cpp/tests/cppcodegentest



- David


On 2009-09-24 15:38:57, Bertjan Broeksema wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1710/
> -----------------------------------------------------------
> 
> (Updated 2009-09-24 15:38:57)
> 
> 
> Review request for KDevelop, Andreas Pakulat, Milian Wolff, and David Nolden.
> 
> 
> Summary
> -------
> 
> Adds a mapping of uses and AST nodes to ParseSession in addition to mappings between DUCHain declarations and nodes. I also factored out some code related to mappings out of DeclarationBuilder into ContextBuilder.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdevelop/languages/cpp/cppduchain/contextbuilder.h 1027714 
>   trunk/KDE/kdevelop/languages/cpp/cppduchain/contextbuilder.cpp 1027714 
>   trunk/KDE/kdevelop/languages/cpp/cppduchain/declarationbuilder.h 1027714 
>   trunk/KDE/kdevelop/languages/cpp/cppduchain/declarationbuilder.cpp 1027714 
>   trunk/KDE/kdevelop/languages/cpp/cppduchain/tests/test_duchain.cpp 1027714 
>   trunk/KDE/kdevelop/languages/cpp/cppduchain/usebuilder.cpp 1027714 
>   trunk/KDE/kdevelop/languages/cpp/cppparsejob.cpp 1027714 
>   trunk/KDE/kdevelop/languages/cpp/parser/parsesession.h 1027714 
>   trunk/KDE/kdevelop/languages/cpp/parser/parsesession.cpp 1027714 
> 
> Diff: http://reviewboard.kde.org/r/1710/diff
> 
> 
> Testing
> -------
> 
> Work with a KDevelop that has this patch applied. No crashes related to this seem to happen. I did not yet test which nodes actually get mapped. Will do that soon.
> 
> 
> Thanks,
> 
> Bertjan
> 
>