Review Request 117509: Use DeclarationBuilder to build declarations before ExpressionVisitor tries to find their type

[Denis Steckelmacher]

> On April 12, 2014, 2:10 p.m., Sven Brauch wrote:
> > duchain/expressionvisitor.cpp, line 41
> > <https://git.reviewboard.kde.org/r/117509/diff/1-2/?file=264915#file264915line41>
> >
> >     Ah, the parser even gives you the string. Then I think searching that for a dot would be fine, and easier to read than the log10 stuff. But you can also keep it like this, I don't care.

The parser only provides the position of the token. The string must be retrieved by using ParseSession::symbolAt. This solution would require that ExpressionVisitor knows about ParseSession, and I don't know if it is possible in the code-completion context.


> On April 12, 2014, 2:10 p.m., Sven Brauch wrote:
> > tests/files/helloworld.js, line 31
> > <https://git.reviewboard.kde.org/r/117509/diff/2/?file=264932#file264932line31>
> >
> >     Hmm, what part of your patch breaks this (if it was working before)?

I removed ExpressionVisitor::endVisit(QmlJS::AST::FunctionExpression*). This method built a simple FunctionType returning void. As ContextBuilder already recognized function expressions and built a context for them and their parameters, the tests passed but is was fragile. My following patches are (in this order, if it suits you) : adding support for "a == b", adding back support for function expressions (using anonymous declarations), adding support for unsure types (in return statements), adding support for variables whose types are known only after their declaration ("function foo(a) { a = 5; }", a is an int), inferring the type of function parameters using their call-sites ("foo(5, 3.14)" takes two parameters: int and double) and then I will start my research about objects and function prototypes :-) . All these patches should be very simple and self-contained.


- Denis


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117509/#review55509
-----------------------------------------------------------


On April 12, 2014, 11:01 a.m., Denis Steckelmacher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117509/
> -----------------------------------------------------------
> 
> (Updated April 12, 2014, 11:01 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdev-qmljs
> 
> 
> Description
> -------
> 
> This patch is a stripped-down version of review #117458. It changes ExpressionVisitor and ContextBuilder::findType in order to use the same architecture as the one used by Ruby. As Javascript expressions can contain declarations (for instance in "var c = function() { var b = 5; return b; }"), ExpressionVisitor can have problems finding the type of such expressions. By first letting DeclarationBuilder recurse in the expression (and build every declaration it encounters), ExpressionVisitor becomes able to see them and to return their type. ExpressionVisitor is also changed to stop visiting nodes the first time it encounters a type it can deduce, as there is no need to recurse all the way to the bottom of very long JS expressions.
> 
> This patch does not add any feature to the JS language support and only changes what is required in order to have ExpressionVisitor leverage DeclarationBuilder to build declarations. This patch also moves code around and improves how double and int numbers are differentiated. I've put these cleanups in this patch because it already rewrites most of ExpressionVisitor, and the next patches are easier to understand if they don't move anything and just add methods.
> 
> 
> Diffs
> -----
> 
>   duchain/CMakeLists.txt 9fab69c 
>   duchain/contextbuilder.cpp d38da2d 
>   duchain/declarationbuilder.h 001c3b2 
>   duchain/declarationbuilder.cpp 98da341 
>   duchain/expressionvisitor.h b4f0851 
>   duchain/expressionvisitor.cpp 766435d 
>   duchain/parsesession.h 0eb2762 
>   duchain/parsesession.cpp dbc4d90 
>   duchain/tests/testdeclarations.cpp 668efaa 
>   tests/files/helloworld.js ff40f1c 
> 
> Diff: https://git.reviewboard.kde.org/r/117509/diff/
> 
> 
> Testing
> -------
> 
> The QML/JS testsuite passes with this patch, with two tests marked as expected to fail. The reason is that function expressions are not yet recognized by the parser, so "var c = function(a) { return a; }" doesn't have a type yet.
> 
> 
> Thanks,
> 
> Denis Steckelmacher
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20140412/15c4ab90/attachment-0001.html>