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