Review Request: Add support for parsing Q_PROPERTY declarations

David Nolden zwabel+reviewboard at gmail.com
Thu May 6 08:17:33 UTC 2010


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


Good work.


/trunk/extragear/sdk/kdevelop/languages/cpp/cppduchain/declarationbuilder.cpp
<http://reviewboard.kde.org/r/3814/#comment5145>

    As milian said, you have to check whether currentDeclaration() and currentDeclaration()->internalContext() are nonzero.



/trunk/extragear/sdk/kdevelop/languages/cpp/cppduchain/declarationbuilder.cpp
<http://reviewboard.kde.org/r/3814/#comment5146>

    Hmm doesn't this mean that, for properties that have no accessor methods defined, the builder will try to resolve them _again_ and _again_ and _again_? Sounds like this could cause a significant efficiency problem.
    
    Also, doesn't this mean that you will potentially only find _one_ of the accessor methods right after it has been defined?
    
    It would be better if you would try to resolve them only exactly _once_, at the point when the current context is being closed, because then all the accessor methods are surely defined (see ContextBuilder::closeContext())



/trunk/extragear/sdk/kdevelop/languages/cpp/parser/parser.cpp
<http://reviewboard.kde.org/r/3814/#comment5144>

    It would be much more efficient to use the IndexedString tokenization that aready exists (see Token::symbol()), and use statically built IndexedStrings for the comparisons, aka. 'static IndexedString readStr("READ"); if(propertyField == readStr) ...'.


- David


On 2010-04-26 10:10:16, Tobias Koenig wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3814/
> -----------------------------------------------------------
> 
> (Updated 2010-04-26 10:10:16)
> 
> 
> Review request for KDevelop and David Nolden.
> 
> 
> Summary
> -------
> 
> Add support for parsing Q_PROPERTY declarations to the c++ parser
> 
> Please review for later inclusion into 4.0.0 release.
> 
> 
> Diffs
> -----
> 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/cppduchain/CMakeLists.txt 1117141 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/cppduchain/declarationbuilder.h 1117141 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/cppduchain/declarationbuilder.cpp 1117141 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/cppduchain/qpropertydeclaration.h PRE-CREATION 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/cppduchain/qpropertydeclaration.cpp PRE-CREATION 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/ast.h 1117141 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/default_visitor.h 1117141 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/default_visitor.cpp 1117141 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/dumptree.cpp 1117141 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/lexer.cpp 1117141 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/parser.h 1117141 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/parser.cpp 1117141 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/tests/test_parser.cpp 1117141 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/tokens.h 1117141 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/tokens.cpp 1117141 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/visitor.h 1117141 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/parser/visitor.cpp 1117141 
>   /trunk/extragear/sdk/kdevelop/languages/cpp/setuphelpers.cpp 1117141 
> 
> Diff: http://reviewboard.kde.org/r/3814/diff
> 
> 
> Testing
> -------
> 
> run unit tests
> 
> 
> Thanks,
> 
> Tobias
> 
>





More information about the KDevelop-devel mailing list