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

[Sven Brauch]

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


Looks mostly good, as already discussed in the other RR. Can you quickly explain why some tests are now marked as expect_fail and how you want to fix that again?

Thanks!


duchain/expressionvisitor.cpp
<https://git.reviewboard.kde.org/r/117509/#comment38619>

    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.



tests/files/helloworld.js
<https://git.reviewboard.kde.org/r/117509/#comment38620>

    Hmm, what part of your patch breaks this (if it was working before)?


- Sven Brauch


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/d478a7c1/attachment.html>